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

Witness virtual scheduling issue #29

Closed
abitmore opened this issue Apr 26, 2016 · 5 comments
Closed

Witness virtual scheduling issue #29

abitmore opened this issue Apr 26, 2016 · 5 comments
Labels

Comments

@abitmore
Copy link
Contributor

Current issues:

  1. if a witness is voted out of top 19, it will never be scheduled for time sharing (unless mass votes change?)
  2. if a running up witness went into miner queue, after come out, it will probably be on the top of running up queue for quite some rounds.

Please check my patch abitmore@24e8185 for possible fix.

Todo:

  1. add hard fork logic
  2. review adjust_witness_votes() to identify potential issues
  3. optimization against finding in a vector

Since I have no much time right now, please make your own decision on what's the best way to fix.

@arhag
Copy link
Contributor

arhag commented Apr 26, 2016

  1. review adjust_witness_votes() to identify potential issues

I believe the required changes may be as simple as changing this line into:

w.virtual_position = std::min(VIRTUAL_SCHEDULE_LAP_LENGTH, w.virtual_position +  w.votes.value * (wso.current_virtual_time - w.virtual_last_update));

Or whatever the fc::uint128 version of std::min is.

@bytemaster
Copy link
Contributor

I think the solution I would adopt is to iterate through the active witness set and recalculate the virtual scheduled time. We want to ensure that the top 19 are ignored when selecting the runner up.

@bytemaster
Copy link
Contributor

Assuming you are in the top 19 and get voted out before your block production slot, then when you are paid it will update your virtual scheduled time. So this bug only impacts witnesses that are in the active round and get voted out after they have produced in the round.

As soon as any one of your supporters converts steem to vests then you will be rescheduled. This means any other witness voting for you, any miner voting for you, or any poster.

I think we can implement a fix for this to happen at the next hardfork, but we will not schedule a hardfork just because of this.

@abitmore
Copy link
Contributor Author

Assuming you are in the top 19 and get voted out before your block production slot, then when you are paid it will update your virtual scheduled time. So this bug only impacts witnesses that are in the active round and get voted out after they have produced in the round.

Found an issue in adjust_witness_votes(). The fix suggested by @arhag in an earlier comment would work.

While a witness is voted in, she will never be scheduled for time sharing, so virtual_position keeps increasing. After virtual_position > VIRTUAL_SCHEDULE_LAP_LENGTH, with this code:

w.virtual_scheduled_time = w.virtual_last_update + (VIRTUAL_SCHEDULE_LAP_LENGTH - w.virtual_position)/(w.votes.value+1);

the virtual scheduled time will be set to a very large value which is converted from a negative value as unit128. As a result, she will never be scheduled after been voted out of top 19.

@abitmore
Copy link
Contributor Author

abitmore commented Apr 27, 2016

It's best to keep virtual_scheduled_time, virtual_last_update and virtual_position consistent, otherwise there would be any unexpected behavior.

Some issues about latest commit of tag 0.3.0:

  • overflow at L934 of database.cpp when wo.votes is 0 (thanks to arhag)
  • lack of a update to wo.virtual_last_update in L922-L937 of database.cpp
  • lack of an reset to sitr in database.cpp before L940, in addition please make sure new_virtual_time is initialized

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

No branches or pull requests

3 participants