From fad67835bd6214559dc3bdb784265893c5f935ea Mon Sep 17 00:00:00 2001 From: David Llop <931684+sembrestels@users.noreply.github.com> Date: Tue, 26 Oct 2021 22:52:50 +0200 Subject: [PATCH 1/6] Critical: do not allow recoverability when the hatch is ongoing (fixes #3) --- contracts/Hatch.sol | 7 +++++++ test/States.test.js | 18 +++++++++++++++++- 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/contracts/Hatch.sol b/contracts/Hatch.sol index c2aee0f..09a0f07 100644 --- a/contracts/Hatch.sol +++ b/contracts/Hatch.sol @@ -250,6 +250,13 @@ contract Hatch is EtherTokenConstant, IsContract, AragonApp, IACLOracle { return vestingCompleteDate != 0 && getTimestamp64() >= vestingCompleteDate; } + /** + * @dev Turns off fund recovery for contribution token when the hatch is ongoing + */ + function allowRecoverability(address _token) public view isInitialized returns (bool) { + return _token != contributionToken || state() == State.Pending || state() == State.Closed; + } + /***** internal functions *****/ function _timeSinceOpen() internal view returns (uint64) { diff --git a/test/States.test.js b/test/States.test.js index 0537823..03d1cac 100644 --- a/test/States.test.js +++ b/test/States.test.js @@ -1,6 +1,6 @@ const { HATCH_PERIOD, HATCH_MAX_GOAL, HATCH_STATE, HATCH_MIN_GOAL } = require('./helpers/constants') const { prepareDefaultSetup, defaultDeployParams, initializeHatch } = require('./common/deploy') -const { now } = require('./common/utils') +const { now, tokenExchangeRate } = require('./common/utils') const getState = async test => { return (await test.hatch.state()).toNumber() @@ -21,6 +21,10 @@ contract('Hatch, states validation', ([anyone, appManager, buyer]) => { assert.equal(await getState(this), HATCH_STATE.PENDING) }) + it('It can escape hatch', async() => { + assert.isTrue(await this.hatch.allowRecoverability(this.contributionToken.address)); + }) + describe('When the sale is started', () => { before(async () => { if (startDate == 0) { @@ -34,6 +38,10 @@ contract('Hatch, states validation', ([anyone, appManager, buyer]) => { assert.equal(await getState(this), HATCH_STATE.FUNDING) }) + it('It cannot escape hatch', async() => { + assert.isFalse(await this.hatch.allowRecoverability(this.contributionToken.address)); + }) + describe('When the funding period is still running', () => { before(async () => { this.hatch.mockSetTimestamp(startDate + HATCH_PERIOD / 2) @@ -60,6 +68,10 @@ contract('Hatch, states validation', ([anyone, appManager, buyer]) => { it('The state is Refunding', async () => { assert.equal(await getState(this), HATCH_STATE.REFUNDING) }) + + it('It cannot escape hatch', async() => { + assert.isFalse(await this.hatch.allowRecoverability(this.contributionToken.address)); + }) }) }) @@ -112,6 +124,10 @@ contract('Hatch, states validation', ([anyone, appManager, buyer]) => { it('The state is Closed', async () => { assert.equal(await getState(this), HATCH_STATE.CLOSED) }) + + it('It can escape hatch', async() => { + assert.isTrue(await this.hatch.allowRecoverability(this.contributionToken.address)); + }) }) }) }) From 2271de8c11a01ad4172034bf7a4f136c7443b572 Mon Sep 17 00:00:00 2001 From: David Llop <931684+sembrestels@users.noreply.github.com> Date: Tue, 26 Oct 2021 23:07:27 +0200 Subject: [PATCH 2/6] Use `.call()` with value instead of `.transfer()` (fixes #4) --- contracts/Hatch.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/Hatch.sol b/contracts/Hatch.sol index 09a0f07..1c19e7c 100644 --- a/contracts/Hatch.sol +++ b/contracts/Hatch.sol @@ -288,7 +288,7 @@ contract Hatch is EtherTokenConstant, IsContract, AragonApp, IACLOracle { function _contribute(address _contributor, uint256 _value) internal { uint256 value = totalRaised.add(_value) > maxGoal ? maxGoal.sub(totalRaised) : _value; if (contributionToken == ETH && _value > value) { - msg.sender.transfer(_value.sub(value)); + msg.sender.call.value(_value.sub(value)); } // (contributor) ~~~> contribution tokens ~~~> (hatch) @@ -369,7 +369,7 @@ contract Hatch is EtherTokenConstant, IsContract, AragonApp, IACLOracle { if (_token == ETH) { require(_from == address(this), ERROR_TOKEN_TRANSFER_REVERTED); require(_to != address(this), ERROR_TOKEN_TRANSFER_REVERTED); - _to.transfer(_amount); + _to.call.value(_amount); } else { if (_from == address(this)) { require(ERC20(_token).safeTransfer(_to, _amount), ERROR_TOKEN_TRANSFER_REVERTED); From 7f70936e962f8943d1e94a765b890f34645d776e Mon Sep 17 00:00:00 2001 From: David Llop <931684+sembrestels@users.noreply.github.com> Date: Wed, 27 Oct 2021 01:04:25 +0200 Subject: [PATCH 3/6] Update aragon/os to v4.4.0 (fixes #9) --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index a9d87ce..71b3b0e 100644 --- a/package.json +++ b/package.json @@ -30,7 +30,7 @@ ], "dependencies": { "@aragon/apps-token-manager": "^2.1.0", - "@aragon/os": "4.2.1" + "@aragon/os": "^4.4.0" }, "devDependencies": { "@aragon/minime": "^1.0.0", From 0157e036e7a65e8cd8246994c423886522e8dffb Mon Sep 17 00:00:00 2001 From: David Llop <931684+sembrestels@users.noreply.github.com> Date: Wed, 27 Oct 2021 01:25:08 +0200 Subject: [PATCH 4/6] Improve performance of `_contribute()` (fixes #12) --- contracts/Hatch.sol | 4 ---- 1 file changed, 4 deletions(-) diff --git a/contracts/Hatch.sol b/contracts/Hatch.sol index 1c19e7c..5518dd0 100644 --- a/contracts/Hatch.sol +++ b/contracts/Hatch.sol @@ -38,8 +38,6 @@ contract Hatch is EtherTokenConstant, IsContract, AragonApp, IACLOracle { string private constant ERROR_INVALID_PCT = "HATCH_INVALID_PCT"; string private constant ERROR_INVALID_STATE = "HATCH_INVALID_STATE"; string private constant ERROR_INVALID_CONTRIBUTE_VALUE = "HATCH_INVALID_CONTRIBUTE_VALUE"; - string private constant ERROR_INSUFFICIENT_BALANCE = "HATCH_INSUFFICIENT_BALANCE"; - string private constant ERROR_INSUFFICIENT_ALLOWANCE = "HATCH_INSUFFICIENT_ALLOWANCE"; string private constant ERROR_NOTHING_TO_REFUND = "HATCH_NOTHING_TO_REFUND"; string private constant ERROR_TOKEN_TRANSFER_REVERTED = "HATCH_TOKEN_TRANSFER_REVERTED"; @@ -293,8 +291,6 @@ contract Hatch is EtherTokenConstant, IsContract, AragonApp, IACLOracle { // (contributor) ~~~> contribution tokens ~~~> (hatch) if (contributionToken != ETH) { - require(ERC20(contributionToken).balanceOf(_contributor) >= value, ERROR_INSUFFICIENT_BALANCE); - require(ERC20(contributionToken).allowance(_contributor, address(this)) >= value, ERROR_INSUFFICIENT_ALLOWANCE); _transfer(contributionToken, _contributor, address(this), value); } // (mint ✨) ~~~> project tokens ~~~> (contributor) From 18a46352315dae9a0973a8f0188db2d4ea60e1c9 Mon Sep 17 00:00:00 2001 From: David Llop <931684+sembrestels@users.noreply.github.com> Date: Wed, 27 Oct 2021 01:37:26 +0200 Subject: [PATCH 5/6] Allow using the same date for start, cliff and complete (fixed #14) --- contracts/Hatch.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/Hatch.sol b/contracts/Hatch.sol index 5518dd0..9e7df58 100644 --- a/contracts/Hatch.sol +++ b/contracts/Hatch.sol @@ -121,8 +121,8 @@ contract Hatch is EtherTokenConstant, IsContract, AragonApp, IACLOracle { require(_maxGoal >= _minGoal, ERROR_INVALID_MAX_GOAL); require(_period > 0, ERROR_INVALID_TIME_PERIOD); require(_exchangeRate > 0, ERROR_INVALID_EXCHANGE_RATE); - require(_vestingCliffPeriod > _period, ERROR_INVALID_TIME_PERIOD); - require(_vestingCompletePeriod > _vestingCliffPeriod, ERROR_INVALID_TIME_PERIOD); + require(_vestingCliffPeriod >= _period, ERROR_INVALID_TIME_PERIOD); + require(_vestingCompletePeriod >= _vestingCliffPeriod, ERROR_INVALID_TIME_PERIOD); require(_supplyOfferedPct > 0 && _supplyOfferedPct <= PPM, ERROR_INVALID_PCT); require(_fundingForBeneficiaryPct >= 0 && _fundingForBeneficiaryPct <= PPM, ERROR_INVALID_PCT); From 8ab1ac2ac7b3ce163472601609f5b933d4d9ccd0 Mon Sep 17 00:00:00 2001 From: David Llop <931684+sembrestels@users.noreply.github.com> Date: Wed, 27 Oct 2021 01:45:36 +0200 Subject: [PATCH 6/6] Optimize storage slots (fixes #15) --- contracts/Hatch.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/Hatch.sol b/contracts/Hatch.sol index 9e7df58..1a31969 100644 --- a/contracts/Hatch.sol +++ b/contracts/Hatch.sol @@ -57,13 +57,13 @@ contract Hatch is EtherTokenConstant, IsContract, AragonApp, IACLOracle { uint256 public minGoal; uint256 public maxGoal; - uint64 public period; uint256 public exchangeRate; + uint64 public period; uint64 public vestingCliffPeriod; uint64 public vestingCompletePeriod; + uint64 public openDate; uint256 public supplyOfferedPct; uint256 public fundingForBeneficiaryPct; - uint64 public openDate; bool public isClosed; uint64 public vestingCliffDate;