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

Improve encapsulation on TimedCrowdsale #1281

Merged
Show file tree
Hide file tree
Changes from 32 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
128c4c0
Improve encapsulation on ERC165
come-maiz Sep 3, 2018
0d8a5e4
Improve encapsulation on ERC20
come-maiz Sep 3, 2018
1c365fe
Improve encapsulation on ERC721
come-maiz Sep 3, 2018
9ebdae6
Add tests, use standard getters
come-maiz Sep 3, 2018
963a00b
fix tests
come-maiz Sep 3, 2018
4d8db8d
Fix lint
come-maiz Sep 3, 2018
dc7b21e
Merge branch 'master' into refactor/1174/private-state-vars-erc
Sep 4, 2018
0b0d6c6
move interface ids to implementation contracts
frangio Sep 4, 2018
68094d1
Do not prefix getters
come-maiz Sep 5, 2018
2ea79e9
Merge branch 'master' into refactor/1174/private-state-vars-erc
come-maiz Sep 5, 2018
5fc3742
Improve encapsulation on Pausable
come-maiz Sep 3, 2018
172b720
add the underscore
come-maiz Sep 3, 2018
86bbab3
Improve encapsulation on ownership
come-maiz Sep 3, 2018
faed52d
fix rebase
come-maiz Sep 3, 2018
21ae177
fix ownership
come-maiz Sep 3, 2018
917a019
Improve encapsulation on payments
come-maiz Sep 3, 2018
478d974
Add missing tests
come-maiz Sep 4, 2018
0fecbac
add missing test
come-maiz Sep 4, 2018
9449572
Do not prefix getters
come-maiz Sep 5, 2018
1856f07
Fix tests.
come-maiz Sep 5, 2018
f61acdc
Improve encapsulation on Crowdsales
come-maiz Sep 3, 2018
44d113a
add missing tests
come-maiz Sep 3, 2018
0257670
remove only
come-maiz Sep 3, 2018
5a476d1
Do not prefix getters
come-maiz Sep 5, 2018
85a0fc3
Update modifiers to call public view functions.
come-maiz Sep 5, 2018
24761a5
remove isMinter
come-maiz Sep 5, 2018
c14d597
fix is owner call
come-maiz Sep 5, 2018
ba7fa16
fix isOpen
come-maiz Sep 5, 2018
776d47a
Merge branch 'master' into feature/1179/modifiers-call-functions
come-maiz Sep 6, 2018
7a37725
Fix merge
come-maiz Sep 6, 2018
52352ce
Improve encapsulation on TimedCrowdsale
come-maiz Sep 6, 2018
63b93cc
Add missing parentheses
come-maiz Sep 6, 2018
b6bddb2
Merge branch 'master' into refactor/1174/private-state-vars-timed-cro…
frangio Sep 6, 2018
369b8d6
remove duplicate function definition
frangio Sep 6, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions contracts/crowdsale/price/IncreasingPriceCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ contract IncreasingPriceCrowdsale is TimedCrowdsale {
*/
function getCurrentRate() public view returns (uint256) {
// solium-disable-next-line security/no-block-members
uint256 elapsedTime = block.timestamp.sub(openingTime);
uint256 timeRange = closingTime.sub(openingTime);
uint256 elapsedTime = block.timestamp.sub(openingTime());
uint256 timeRange = closingTime().sub(openingTime());
uint256 rateRange = initialRate_.sub(finalRate_);
return initialRate_.sub(elapsedTime.mul(rateRange).div(timeRange));
}
Expand Down
34 changes: 28 additions & 6 deletions contracts/crowdsale/validation/TimedCrowdsale.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ import "../Crowdsale.sol";
contract TimedCrowdsale is Crowdsale {
using SafeMath for uint256;

uint256 public openingTime;
uint256 public closingTime;
uint256 private openingTime_;
uint256 private closingTime_;

/**
* @dev Reverts if not in crowdsale time range.
*/
modifier onlyWhileOpen {
// solium-disable-next-line security/no-block-members
require(block.timestamp >= openingTime && block.timestamp <= closingTime);
require(isOpen());
_;
}

Expand All @@ -33,8 +33,30 @@ contract TimedCrowdsale is Crowdsale {
require(_openingTime >= block.timestamp);
require(_closingTime >= _openingTime);

openingTime = _openingTime;
closingTime = _closingTime;
openingTime_ = _openingTime;
closingTime_ = _closingTime;
}

/**
* @return the crowdsale opening time.
*/
function openingTime() public view returns(uint256) {
return openingTime_;
}

/**
* @return the crowdsale closing time.
*/
function closingTime() public view returns(uint256) {
return closingTime_;
}

/**
* @return true if the crowdsale is open, false otherwise.
*/
function isOpen() public view returns (bool) {
// solium-disable-next-line security/no-block-members
return block.timestamp >= openingTime_ && block.timestamp <= closingTime_;
}

/**
Expand All @@ -43,7 +65,7 @@ contract TimedCrowdsale is Crowdsale {
*/
function hasClosed() public view returns (bool) {
// solium-disable-next-line security/no-block-members
return block.timestamp > closingTime;
return block.timestamp > closingTime_;
}

/**
Expand Down
9 changes: 8 additions & 1 deletion contracts/ownership/Ownable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,17 @@ contract Ownable {
* @dev Throws if called by any account other than the owner.
*/
modifier onlyOwner() {
require(msg.sender == owner_);
require(isOwner());
_;
}

/**
* @return true if `msg.sender` is the owner of the contract.
*/
function isOwner() public view returns(bool) {
return msg.sender == owner_;
}

/**
* @dev Allows the current owner to relinquish control of the contract.
* @notice Renouncing to ownership will leave the contract without an owner.
Expand Down
17 changes: 11 additions & 6 deletions contracts/token/ERC20/ERC20Mintable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ contract ERC20Mintable is ERC20, Ownable {
bool private mintingFinished_ = false;


modifier canMint() {
modifier onlyBeforeMintingFinished() {
require(!mintingFinished_);
_;
}

modifier hasMintPermission() {
require(msg.sender == owner());
modifier onlyMinter() {
require(isOwner());
_;
}

Expand All @@ -44,8 +44,8 @@ contract ERC20Mintable is ERC20, Ownable {
uint256 _amount
)
public
hasMintPermission
canMint
onlyMinter
onlyBeforeMintingFinished
returns (bool)
{
_mint(_to, _amount);
Expand All @@ -57,7 +57,12 @@ contract ERC20Mintable is ERC20, Ownable {
* @dev Function to stop minting new tokens.
* @return True if the operation was successful.
*/
function finishMinting() public onlyOwner canMint returns (bool) {
function finishMinting()
public
onlyOwner
onlyBeforeMintingFinished
returns (bool)
{
mintingFinished_ = true;
emit MintFinished();
return true;
Expand Down
2 changes: 1 addition & 1 deletion contracts/token/ERC20/RBACMintableToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ contract RBACMintableToken is ERC20Mintable, RBAC {
/**
* @dev override the Mintable token modifier to add role based logic
*/
modifier hasMintPermission() {
modifier onlyMinter() {
checkRole(msg.sender, ROLE_MINTER);
_;
}
Expand Down
3 changes: 3 additions & 0 deletions test/crowdsale/TimedCrowdsale.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,17 +52,20 @@ contract('TimedCrowdsale', function ([_, investor, wallet, purchaser]) {
it('should be ended only after end', async function () {
(await this.crowdsale.hasClosed()).should.equal(false);
await increaseTimeTo(this.afterClosingTime);
(await this.crowdsale.isOpen()).should.equal(false);
(await this.crowdsale.hasClosed()).should.equal(true);
});

describe('accepting payments', function () {
it('should reject payments before start', async function () {
(await this.crowdsale.isOpen()).should.equal(false);
await expectThrow(this.crowdsale.send(value), EVMRevert);
await expectThrow(this.crowdsale.buyTokens(investor, { from: purchaser, value: value }), EVMRevert);
});

it('should accept payments after start', async function () {
await increaseTimeTo(this.openingTime);
(await this.crowdsale.isOpen()).should.equal(true);
await this.crowdsale.send(value);
await this.crowdsale.buyTokens(investor, { value: value, from: purchaser });
});
Expand Down
3 changes: 3 additions & 0 deletions test/ownership/Ownable.behavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,11 @@ function shouldBehaveLikeOwnable (owner, [anyone]) {
});

it('changes owner after transfer', async function () {
(await this.ownable.isOwner({ from: anyone })).should.be.equal(false);
await this.ownable.transferOwnership(anyone, { from: owner });

(await this.ownable.owner()).should.equal(anyone);
(await this.ownable.isOwner({ from: anyone })).should.be.equal(true);
});

it('should prevent non-owners from transfering', async function () {
Expand Down