Tested with BitcoindComparisonTool (which generates a large fork) and -alertnotify script successfully notified me and I get the relevant warnings in getinfo
You’ll get the usually cryptic error: “errors” : “Warning: Displayed transactions may not be correct! You may need to upgrade, or other nodes may need to upgrade.” and alertnotify will be called with Warning: Large-work fork detected. You may need to upgrade, or other nodes may need to upgrade.
I think this code belongs in main rather than alert (rationale: it needs access to the current chainstate, which is maintained by main - alert shouldn’t need a dependency on that).
Code in general looks good, but needs a rebase.
Also, I expect the most common case for a message like this to be a locally corrupted database. Maybe that is worth mentioning is the text?
Such a fork is defined as being at least 7 blocks long and
having a tip which is within 72 blocks of our best block.
1440+ break;
1441+ pfork = pfork->pprev;
1442+ }
1443+
1444+ // We define a condition which we should warn the user about as a fork of at least 7 blocks
1445+ // who's tip is within 72 blocks (+/- 12 hours if no one mines it) of ours
RE: warning during reindex: suppressing the alert if best-other-tip-time is more than a day (a week? or maybe if max(other-tip-time, best-tip-time) is…) in the past is probably the right thing to do: reasoning would be we don’t care about old fork events that have long-since been resolved.
But before implementing that, can we get a block index that demonstrates false positives during a reindex? (I’ve got a corrupt chain that I’ll copy and then reindex if somebody else doesn’t beat me to it).
I removed these three patches from litecoin-0.8.x not because it was broken, but from concern that it would unnecessarily scare people with false positives that appeared to me during a normal reindex. Although I am not entirely certain it was the type of false positive you are asking for. I didn’t investigate too hard as it seemed best to simply remove the patches for the upcoming release meant to minimize risk.
https://github.com/litecoin-project/litecoin/commits/exp-mark11b litecoin-0.8.3.6 is comprised of a merge of the following two branches https://github.com/litecoin-project/litecoin/commits/exp-mark11 litecoin specific commits on top of bitcoin-0.8.3 https://github.com/litecoin-project/litecoin/commits/exp-btc09backports Select bitcoin-0.9 cherry-picks applied on top of bitcoin-0.8.3
https://github.com/litecoin-project/litecoin/commit/286c31b45450eee0817de1ab7704c0911965eeb9 https://github.com/litecoin-project/litecoin/commit/76cf1ab0802677a000aad8188667f1f3345baf24 https://github.com/litecoin-project/litecoin/commit/e751fad2f78bcef601a601cc755809e98c7f0821 Add these back to litecoin-0.8.3.6 and -reindex to see noisy warnings.
https://github.com/litecoin-project/litecoin/tree/exp-forkalert FYI: I tested it yesterday after re-applying the three patches above (an older version of this PR that applied cleanly to bitcoin-0.8.3, prior to the recent rebase). A fresh Litecoin blockchain sync and a subsequent -reindex both were uneventful.
A month ago I saw loud and scary warnings during -reindex twice in a row. I don’t know what is different now. Would a fresh sync with additional checkpoints result in a different local blockchain sans historic forks? Whatever the issue, I am unable to reproduce it with this freshly synced chain now.
#2658 (comment) A month ago I had a corrupted database that exhibited a great many errors in the log but did not crash. sipa’s post immediately before it said, “Also, I expect the most common case for a message like this to be a locally corrupted database. Maybe that is worth mentioning is the text?”
Was his question addressed?