index: Improve robustness of coinstatsindex at restart - #24133
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
|
Concept ACK. I was just able to locally reproduce the fails reported in #24076 and find the root cause - will report in more detail / possibly PR a fix tomorrow. This additional check makes sense in any case. |
|
ACK 2fc159b02187a57980c67268cbf70b9998caa488 - I verified that init aborts now for a corrupted index, and doesn't abort for an uncorrupted one. |
Sjors
left a comment
Sjors
left a comment
There was a problem hiding this comment.
ACK 2fc159b02187a57980c67268cbf70b9998caa488
Maybe also annotate MuHash3072 m_muhash; in the header to point out a MuHash is initialized to 1 by default.
2fc159b to
820c03a
Compare
|
Rebased |
|
re-ACK 820c03a |
1 similar comment
|
re-ACK 820c03a |
| uint256 out; | ||
| m_muhash.Finalize(out); | ||
| if (entry.muhash != out) { | ||
| return error("%s: Cannot read current %s state; index may be corrupted", |
There was a problem hiding this comment.
In commit "index: check muhash is in sync on coinstatsindex launch" (820c03a)
I wonder if it would be good to include hashes or other information in this message (transaction total numbers, pindex height) in the error message, especially if this is supposed to help debug an unresolved issue.
There was a problem hiding this comment.
re: #24133 (comment)
I wonder if it would be good to include hashes or other information in this message
Never mind if #24138 fixes this though
There was a problem hiding this comment.
I will address this together with your idea for a unit test here in a small follow-up.
7 participants
This change lets the
coinstatsindexfail loudly in case the internalmuhashstate differs from the last finalized output saved on disk, which would indicate that themuhashstate somehow got out of sync. This should generally not happen since both are written to disk in a batch but #24076 seems to indicate that the might still be an issue.Since #24076 so far can not be reproduced reliably, the issue should not be closed yet. Further investigation and testing needs to be done.