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

Fix potential div-by-zero issue in receive_frontier #1629

Merged

Conversation

Projects
3 participants
@cryptocode
Copy link
Collaborator

commented Jan 24, 2019

The first time received_frontier is called, it's only measuring the time of count++. Since the standard only guarantees non-decreasing values for steady_clock, it looks like the value could be zero on systems with low timer resolution, causing (since it's a double) NaN, +/-inf or undefined behavior if !is_iec559. Not likely, but a guard seems good here anyway. The approach is consistent with that of bootstrap_client::block_rate ()

@cryptocode cryptocode self-assigned this Jan 24, 2019

@cryptocode cryptocode added the bug label Jan 24, 2019

@cryptocode cryptocode added this to the V18.0 milestone Jan 24, 2019

@cryptocode cryptocode added this to During RC in V18 Jan 24, 2019

@@ -294,7 +294,7 @@ void nano::frontier_req_client::received_frontier (boost::system::error_code con
++count;
std::chrono::duration<double> time_span = std::chrono::duration_cast<std::chrono::duration<double>> (std::chrono::steady_clock::now () - start_time);
double elapsed_sec = time_span.count ();
double blocks_per_sec = (double)count / elapsed_sec;
double blocks_per_sec = elapsed_sec > 0.0 ? (static_cast<double> (count) / elapsed_sec) : 0.0;

This comment has been minimized.

Copy link
@clemahieu

clemahieu Jan 26, 2019

Collaborator

Since a divide by zero case would go toward infinity instead of toward zero, should we pick a minimum value for elapsed seconds? Perhaps a typical timeslice preemption interval, 20ms or 50ms if I recall?

This comment has been minimized.

Copy link
@cryptocode

cryptocode Jan 26, 2019

Author Collaborator

Pushed an updated and, gave bootstrap_client::block_rate the same treatment. Is that in line with your idea? If so, maybe we should factor out a utility function for this.

@clemahieu

This comment has been minimized.

Copy link
Collaborator

commented Jan 26, 2019

Seems like the other one would make sense to change in the same way yea. It’s just weird that a lower and lower elapsed number will let the rate spiral upward, unless it’s zero and then it’s discontinious at 0

@cryptocode cryptocode merged commit e7b685a into nanocurrency:master Jan 30, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zhyatt zhyatt moved this from RC2 to CP 3/RC 1 (2018-02-01) in V18 Feb 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.