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

total_missed does not correctly increment in a corner case #2469

Closed
theoreticalbts opened this issue May 18, 2018 · 1 comment
Closed

total_missed does not correctly increment in a corner case #2469

theoreticalbts opened this issue May 18, 2018 · 1 comment
Assignees

Comments

@theoreticalbts
Copy link
Contributor

This code skips updating total_missed in case witness_missed.owner == b.owner. To fix the bug, transform from:

if( witness_missed.owner != b.witness )
{
   modify(witness_missed, [&](witness_object& w)
   {
      w.total_missed++;
      if( has_hf_14 ) { if( last_old_enough ) { shutdown_witness } }
   }
}

to:

modify(witness_missed, [&](witness_object& w)
{
   w.total_missed++;
   if( witness_missed.owner != b.witness )
   {
      if( has_hf_14 ) { if( last_old_enough ) { shutdown_witness } }
   }
}

We can collapse these three checks to a single if statement with a conjunction (AND) of these three conditions, reordered any we like, and I like this ordering:

modify(witness_missed, [&](witness_object& w)
{
   w.total_missed++;
   if( last_old_enough
     && witness_missed.owner != b.witness
     && has_hf_14 )
   { shutdown_witness }
}

The idea behind this reordering is that, by deferring the hardfork check (involving an object lookup) until the last, we might well avoid it entirely if one of the other checks fails (the other two checks are both only integer arithmetic and reading fields from objects already available).

I'd really like to get rid of the witness_missed.owner != b.owner check, but I realized we cannot remove it, otherwise the network might be bricked if all of the witnesses miss enough blocks to get shut down, then start producing again.

@theoreticalbts
Copy link
Contributor Author

@abitmore raises the point that this patch might be "cruel" to some witnesses by increasing reported total_missed. For that reason, we've decided not to change the functionality.

I will therefore close this ticket and PR #2472, and remove this commit from PR #2473.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant