Page MenuHomePhabricator

[ibd.sh] Add chainwork sanity check
AbandonedPublic

Authored by jasonbcox on Jun 28 2019, 00:23.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Summary

This provides an additional guarantee that IBD reached its target chainwork.
These sorts of sanity checks are crucial for debugging since IBD takes a long time to complete.

In the future, we may want a more robust mechanism for fetching nMinimumChainWork, but fetching it from the logs will do for now.

Depends on D3452

Test Plan

IBD on TeamCity

Diff Detail

Repository
rABC Bitcoin ABC
Branch
ibd-chainparams
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6526
Build 11099: Bitcoin ABC Buildbot (legacy)
Build 11098: arc lint + arc unit

Event Timeline

Added missing datadir to bitcoin-cli calls

deadalnix requested changes to this revision.Jun 30 2019, 00:00

The test plan doesn't test the feature. For all we know this could fail to extract the proper value for chain work and just copare to zero or something and you wouldn't know it.

This revision now requires changes to proceed.Jun 30 2019, 00:00
  • Fixed a bug where chainparams wouldn't compare as expected.
  • Make the success of this check visually inspectable.
deadalnix requested changes to this revision.Jul 2 2019, 01:37
deadalnix added inline comments.
contrib/teamcity/ibd.sh
46 ↗(On Diff #9892)

That is exactly why you need a dedicated header for this. You know you do, you even moved in that direction in another diff. So why this?

60 ↗(On Diff #9892)

I'm not sure what this is supposed to achieve. Shouldn't this be a test in the software to make sure IBD is never marked as done if we haven't reached the minimum chain work?

If it is already tested, then this is redundant, and if it isn't then why test it here rather than there?

This revision now requires changes to proceed.Jul 2 2019, 01:37