-
Notifications
You must be signed in to change notification settings - Fork 72
feat: add countdown log till genesis #1104
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
feat: add countdown log till genesis #1104
Conversation
bin/ream/src/main.rs
Outdated
| let now = SystemTime::now() | ||
| .duration_since(UNIX_EPOCH) | ||
| .expect("System time is before UNIX epoch") | ||
| .as_secs(); |
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.
Isn't this expect() comment misleading?, I think we should avoid expect here, handle the exception intentionally and safely
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.
Referenced from here.
| .expect("System time is before UNIX epoch") |
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.
Yeah I will be removing those in a PR I am working on
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.
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or(Duration::MAX)// remaining <= 0 will be true if error
.as_secs();
let remaining = lean_network_spec().genesis_time - now;
if remaining <= 0 {
info!("Genesis reached! Starting services...");
break;
}Does this look ok? Or should safely log error for that specific case?
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.
let remaining = lean_network_spec().genesis_time.saturating_sub(now);
You should do a saturating sub then to avoid underflows
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.
let now = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap_or(Duration::MAX)
.as_secs();
let remaining = lean_network_spec().genesis_time.saturating_sub(now);
if remaining == 0 {
info!("Genesis reached! Starting services...");
break;
}If this looks okay, then will update the PR.
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.
Remove the // remaining <= 0 will be true if error
also if now == Duration::MAX we should not print anything and just immediately break
So maybe do
let now = match SystemTime::now()
.duration_since(UNIX_EPOCH) {
Ok(now) => now.as_secs(),
// The chain is already past genesis, we don't need a countdown
Err(_) => break,
};
let remaining = lean_network_spec().genesis_time.saturating_sub(now);
if remaining == 0 {
info!("Genesis reached! Starting services...");
break;
}
^ so do something like this
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 didn't suggest that looking at the full context, so you might need to make further modifications
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.
Thanks.
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 tested.
bin/ream/src/main.rs
Outdated
| let now = match SystemTime::now().duration_since(UNIX_EPOCH) { | ||
| Ok(now) => now.as_secs(), | ||
| // The chain is already past genesis, we don't need a countdown | ||
| Err(_) => break, | ||
| }; | ||
|
|
||
| let remaining = lean_network_spec().genesis_time.saturating_sub(now); | ||
|
|
||
| if remaining == 0 { | ||
| info!("Genesis reached! Starting services..."); | ||
| break; | ||
| } |
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.
| let now = match SystemTime::now().duration_since(UNIX_EPOCH) { | |
| Ok(now) => now.as_secs(), | |
| // The chain is already past genesis, we don't need a countdown | |
| Err(_) => break, | |
| }; | |
| let remaining = lean_network_spec().genesis_time.saturating_sub(now); | |
| if remaining == 0 { | |
| info!("Genesis reached! Starting services..."); | |
| break; | |
| } | |
| let now = SystemTime::now() | |
| .duration_since(UNIX_EPOCH) | |
| .unwrap_or(Duration::MAX) | |
| .as_secs(); | |
| let genesis = lean_network_spec().genesis_time; | |
| if now >= genesis { | |
| if now <= genesis + 2 { | |
| info!("Genesis reached! Starting services..."); | |
| } | |
| break; | |
| } |
After reading the code a bit more, could you do this ^

What was wrong?
#886
How was it fixed?
#1104
To-Do
Review