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

TokenVesting improvements #1431

Merged
merged 7 commits into from Oct 18, 2018
Merged
@@ -16,8 +16,8 @@ contract TokenVesting is Ownable {
using SafeMath for uint256;
using SafeERC20 for IERC20;

event Released(uint256 amount);
event Revoked();
event TokensReleased(address token, uint256 amount);
event TokenVestingRevoked(address token);

// beneficiary of tokens after they are released
address private _beneficiary;
@@ -52,6 +52,8 @@ contract TokenVesting is Ownable {
{
require(beneficiary != address(0));
require(cliffDuration <= duration);
require(duration > 0);
require(start.add(duration) > block.timestamp);

This comment has been minimized.

Copy link
@elopio

elopio Oct 17, 2018

Member

Isn't it weird that start can be a time in the past?

This comment has been minimized.

Copy link
@nventuro

nventuro Oct 17, 2018

Author Member

Yes, but I think it may make sense in some scenarios, so I think it's fine.


_beneficiary = beneficiary;
_revocable = revocable;
@@ -122,7 +124,7 @@ contract TokenVesting is Ownable {

token.safeTransfer(_beneficiary, unreleased);

emit Released(unreleased);
emit TokensReleased(token, unreleased);
}

/**
@@ -143,7 +145,7 @@ contract TokenVesting is Ownable {

token.safeTransfer(owner(), refund);

emit Revoked();
emit TokenVestingRevoked(token);
}

/**
@@ -1,4 +1,5 @@
const shouldFail = require('../helpers/shouldFail');
const expectEvent = require('../helpers/expectEvent');
const time = require('../helpers/time');
const { ethGetBlock } = require('../helpers/web3');
const { ZERO_ADDRESS } = require('../helpers/constants');
@@ -22,7 +23,7 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
this.duration = time.duration.years(2);
});

it('rejects a duration shorter than the cliff', async function () {
it('reverts with a duration shorter than the cliff', async function () {
const cliffDuration = this.duration;
const duration = this.cliffDuration;

@@ -33,12 +34,28 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
);
});

it('requires a valid beneficiary', async function () {
it('reverts with a null beneficiary', async function () {
await shouldFail.reverting(
TokenVesting.new(ZERO_ADDRESS, this.start, this.cliffDuration, this.duration, true, { from: owner })
);
});

it('reverts with a null duration', async function () {
// cliffDuration should also be 0, since the duration must be larger than the cliff
await shouldFail.reverting(
TokenVesting.new(beneficiary, this.start, 0, 0, true, { from: owner })
);
});

it('reverts if the end time is in the past', async function () {
const now = await time.latest();

this.start = now - this.duration - time.duration.minutes(1);
await shouldFail.reverting(
TokenVesting.new(beneficiary, this.start, this.cliffDuration, this.duration, true, { from: owner })
);
});

context('once deployed', function () {
beforeEach(async function () {
this.vesting = await TokenVesting.new(
@@ -62,7 +79,11 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {

it('can be released after cliff', async function () {
await time.increaseTo(this.start + this.cliffDuration + time.duration.weeks(1));
await this.vesting.release(this.token.address);
const { logs } = await this.vesting.release(this.token.address);
expectEvent.inLogs(logs, 'TokensReleased', {
token: this.token.address,
amount: await this.token.balanceOf(beneficiary),
});
});

it('should release proper amount after cliff', async function () {
@@ -100,7 +121,8 @@ contract('TokenVesting', function ([_, owner, beneficiary]) {
});

it('should be revoked by owner if revocable is set', async function () {
await this.vesting.revoke(this.token.address, { from: owner });
const { logs } = await this.vesting.revoke(this.token.address, { from: owner });
expectEvent.inLogs(logs, 'TokenVestingRevoked', { token: this.token.address });
(await this.vesting.revoked(this.token.address)).should.equal(true);
});

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.