-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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: Do not use nHeight when trying to identify the very first/initial snapshot #5538
Conversation
concept ACK. |
That's what I thought initially too but looking at logs a bit more - node1 was at height=14 already when it crashed, it must be some deeper issue. |
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.
ACK
…l snapshot `-1` will only mean `not initialized` from now
I think I found the root of it (pls check PR description). |
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.
LGTM
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.
utACK for squash merge
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.
LGTM
Issue being fixed or feature implemented
-1
will only meannot initialized
from nowShould fix crashes like https://gitlab.com/dashpay/dash/-/jobs/4893359925
This fix applied on top of #5525 https://gitlab.com/UdjinM6/dash/-/pipelines/972154075
What was done?
Introduce and use
m_initial_snapshot_index
instead of re-usingnHeight
. Added a couple of asserts to make sure:nHeight
set to-1
explicitly (but it's ok for ctor with no params to do so)nHeight
to-1
for an existing mn listGetListForBlockInternal
never returns non-initialized mn listsHow Has This Been Tested?
Run tests, run regtest/testnet wallets.
Breaking Changes
We never stored snapshots with
nHeight == -1
, should be no breaking changes I think.Checklist: