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 crash when announcing votes #1501

Merged
merged 2 commits into from Dec 28, 2018

Conversation

Projects
None yet
6 participants
@wezrule
Copy link
Collaborator

commented Dec 26, 2018

This was found using MSVC 2017 in a debug build; it's possible it happens with others.

When votes are being announced, there is a loop which traverses all the representatives. While iterating it checks if the current representative is inside rep_votes, if it is then it takes the last element of the representative vector and swaps it with the current one being iterated; the last element is then discarded. It then continues iterating using the newly swapped representative. This works fine until we are unfortunate enough to be iterating over the last element of the vector which also happens to be inside rep_votes. It gets popped off and our current iterator (j) becomes invalidated. The node then falls over when the next (j != m) condition statement is reached. I now check whether we are at the end of the vector and break out early if so.

I'm not familiar enough with the codebase to be able to reproduce this easily or create a suitable test for it (if someone is then feel free!). I just delete the Raiblocks user folder and start a fresh node from the live network, wait and hope for the best (or worst in this case).

@SergiySW SergiySW requested review from cryptocode and SergiySW Dec 26, 2018

@cryptocode
Copy link
Collaborator

left a comment

LGTM

@zhyatt zhyatt added the bug label Dec 27, 2018

@zhyatt zhyatt added this to the V18.0 milestone Dec 28, 2018

@zhyatt zhyatt added this to Unscheduled in V18 Dec 28, 2018

@rkeene rkeene merged commit 074ba48 into nanocurrency:master Dec 28, 2018

2 checks passed

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

@SergiySW SergiySW moved this from Unscheduled to CP 0 in V18 Dec 28, 2018

@wezrule wezrule deleted the wezrule:fix_windows_crash_bootstrapping branch Jan 8, 2019

@zhyatt zhyatt modified the milestones: V18.0, V17.1 Jan 16, 2019

@zhyatt zhyatt removed this from CP 0 in V18 Jan 16, 2019

rkeene added a commit that referenced this pull request Jan 17, 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.