[ibd.sh] Add chainwork sanity check
AbandonedPublic
Actions

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

Details

Reviewers

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 6540
Build 11127: Bitcoin ABC Buildbot (legacy)
Build 11126: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Jun 28 2019, 00:23

Herald added a reviewer: deadalnix. · View Herald TranscriptJun 28 2019, 00:23

jasonbcox planned changes to this revision.Jun 28 2019, 00:23

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

jasonbcox mentioned this in D3452: [ibd.sh] Store datadir and debug.log paths in variables.Jun 28 2019, 00:23

jasonbcox edited the summary of this revision. (Show Details)

jasonbcox added a parent revision: D3452: [ibd.sh] Store datadir and debug.log paths in variables.

Build Bitcoin-ABC / Master / Gitian Builds / Bitcoin-ABC IBD Diff Testing (w/o checkpoints) started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6526: Diff 9747.Jun 28 2019, 00:32

Build Bitcoin-ABC / Master / Gitian Builds / Bitcoin-ABC IBD Diff Testing (w/o checkpoints) failed.

Added missing datadir to bitcoin-cli calls

jasonbcox planned changes to this revision.Jun 28 2019, 16:06

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B6540: Diff 9774.Jun 28 2019, 16:17

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Build Bitcoin-ABC / Master / Gitian Builds / Bitcoin-ABC IBD Diff Testing (w/o checkpoints) started.

Build Bitcoin-ABC / Master / Gitian Builds / Bitcoin-ABC IBD Diff Testing (w/o checkpoints) passed.

jasonbcox requested review of this revision.Jun 29 2019, 15:30

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.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B6610: Diff 9892.Jul 1 2019, 23:45

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?