-
Notifications
You must be signed in to change notification settings - Fork 11.8k
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
Remove moment.js dependencies #368
Changes from 3 commits
99e0f5b
e86ac90
17cfb0f
0d6846a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,7 +32,7 @@ export default function increaseTime(duration) { | |
* @param target time in seconds | ||
*/ | ||
export function increaseTimeTo(target) { | ||
let now = latestTime().unix(); | ||
let now = latestTime(); | ||
if (target < now) throw Error(`Cannot increase current time(${now}) to a moment in the past(${target})`); | ||
let diff = target - now; | ||
return increaseTime(diff); | ||
|
@@ -43,5 +43,7 @@ export const duration = { | |
minutes: function(val) { return val * this.seconds(60) }, | ||
hours: function(val) { return val * this.minutes(60) }, | ||
days: function(val) { return val * this.hours(24) }, | ||
weeks: function(val) { return val * this.days(7) } | ||
weeks: function(val) { return val * this.days(7) }, | ||
months: function(val) { return val * this.days(30)}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm thinking we should only provide time units matching those of Solidity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it makes a total sense to mimic the solidity constants. Thanks for pointing that out! |
||
years: function(val) { return val * this.days(365)} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,4 @@ | ||
import moment from 'moment' | ||
|
||
// Returns a moment.js instance representing the time of the last mined block | ||
// Returns the time of the last mined block in seconds | ||
export default function latestTime() { | ||
return moment.unix(web3.eth.getBlock('latest').timestamp) | ||
return web3.eth.getBlock('latest').timestamp; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm surprised this works. Given the lack of precision of
increaseTimeTo
that we've discussed, I would've thought 3 seconds was not enough to make up for it. Did you try other values?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As
testrpc
is run on localhost the lag is usually up to 1s. Due to time rounding, subtracting a single second is not enough to have a robust behaviour but 3s seems to be safe enough.Obviously, the situation may change if someone decides to run tests across
testrpc
instance connected. But, to be honest, I don't expect it to be a common practice.