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

Uint and Time helpers #375

Merged
merged 8 commits into from
Aug 10, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
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
12 changes: 2 additions & 10 deletions contracts/common/Initializable.sol
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,10 @@
pragma solidity ^0.4.18;

import "../apps/AppStorage.sol";
import "../lib/misc/TimeHelpers.sol";


contract Initializable is AppStorage {
contract Initializable is AppStorage, TimeHelpers {
modifier onlyInit {
require(initializationBlock == 0);
_;
Expand All @@ -31,13 +32,4 @@ contract Initializable is AppStorage {
function initialized() internal onlyInit {
initializationBlock = getBlockNumber();
}

/**
* @dev Returns the current block number.
* Using a function rather than `block.number` allows us to easily mock the block number in
* tests.
*/
function getBlockNumber() internal view returns (uint256) {
return block.number;
}
}
44 changes: 44 additions & 0 deletions contracts/lib/misc/TimeHelpers.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* SPDX-License-Identitifer: MIT
*/

pragma solidity ^0.4.18;


contract TimeHelpers {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Last thing 😅, I'd move this to contracts/common since it's not imported / copied from somewhere else (it's similar to IsContract, which also lives there).

/**
* @dev Returns the current block number.
* Using a function rather than `block.number` allows us to easily mock the block number in
* tests.
*/
function getBlockNumber() internal view returns (uint256) {
return block.number;
}

/**
* @dev Returns the current block number, converted to uint64.
* Using a function rather than `block.number` allows us to easily mock the block number in
* tests.
*/
function getBlockNumber64() internal view returns (uint64) {
return uint64(block.number);
}

/**
* @dev Returns the current timestamp.
* Using a function rather than `now` allows us to easily mock it in
* tests.
*/
function getTimestamp() internal view returns (uint256) {
return now;
}

/**
* @dev Returns the current timestamp, covnerted to uint64.
* Using a function rather than `now` allows us to easily mock it in
* tests.
*/
function getTimestamp64() internal view returns (uint64) {
return uint64(now);
}
}
11 changes: 11 additions & 0 deletions contracts/lib/misc/Uint256Helpers.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pragma solidity ^0.4.18;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this file be in the common directory as well?



library Uint256Helpers {
uint256 public constant MAX_UINT64 = uint64(-1);

function toUint64(uint256 a) internal pure returns (uint64) {
require(a <= MAX_UINT64);
return uint64(a);
}
}
21 changes: 21 additions & 0 deletions test/lib_uint256_helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
const { assertRevert } = require('./helpers/assertThrow')

contract('Uint256 Helpers test', accounts => {
let uint256Mock

before(async () => {
uint256Mock = await artifacts.require('Uint256Mock').new()
})

it('converts from uint256 to uint64', async () => {
const a = 1234
assert.equal((await uint256Mock.convert.call(a)).toString(), a, "Values should match")
})

it('fails converting from uint256 to uint64 if too big', async () => {
const a = new web3.BigNumber(2).pow(64)
return assertRevert(async () => {
await uint256Mock.convert(a)
})
})
})
22 changes: 22 additions & 0 deletions test/mocks/TimeHelpersMock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
pragma solidity 0.4.18;

import "../../contracts/lib/misc/TimeHelpers.sol";


contract TimeHelpersMock is TimeHelpers {
function getBlockNumberExt() public view returns (uint256) {
return getBlockNumber();
}

function getBlockNumber64Ext() public view returns (uint64) {
return getBlockNumber64();
}

function getTimestampExt() public view returns (uint256) {
return getTimestamp();
}

function getTimestamp64Ext() public view returns (uint64) {
return getTimestamp64();
}
}
12 changes: 12 additions & 0 deletions test/mocks/Uint256Mock.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
pragma solidity 0.4.18;

import "../../contracts/lib/misc/Uint256Helpers.sol";


contract Uint256Mock {
using Uint256Helpers for uint256;

function convert(uint256 a) public pure returns (uint64) {
return a.toUint64();
}
}
15 changes: 15 additions & 0 deletions test/time_helpers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
contract('TimeHelpers test', accounts => {
let timeHelpersMock

before(async () => {
timeHelpersMock = await artifacts.require('TimeHelpersMock').new()
})

it('checks block number', async () => {
assert.equal((await timeHelpersMock.getBlockNumberExt.call()).toString(), (await timeHelpersMock.getBlockNumber64Ext.call()).toString(), "block number should match")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also test that the returned value is the actual timestamp or block number.

})

it('checks time stamp', async () => {
assert.equal((await timeHelpersMock.getTimestampExt.call()).toString(), (await timeHelpersMock.getTimestamp64Ext.call()).toString(), "time stamp should match")
})
})