Improve error handling during validation #2224

pull sipa wants to merge 5 commits into bitcoin:master from sipa:valstate changing 10 files +309 −249
  1. sipa commented at 12:53 AM on January 27, 2013: member

    The goal of this pull request is improving how errors during block and transaction validation are propagated, displayed and handled.

    It introduces CValidationState, which stores metadata about a block or transaction validation being performed. It is used to distinguish validation errors (failure to meet network rules, for example) with runtime errors (like out of disk space), as formerly these could be confused, leading to blocks being marked invalid because the disk space ran out. Additionally, CValidationState also takes over the role of tracking DoS levels (so it doesn't need to be stored inside transactions or blocks...).

    Additionally, some extra checks are introduced, excessive coinbase values are made a DoSable offence, disk space is checked before trying to flush the coin case, and read/write errors cause a fatal error (reported to stdout/GUI, followed by shutdown).

  2. luke-jr commented at 1:08 AM on January 27, 2013: member

    Isn't this redundant with #1816 / 46888e6abca27dd6d2132aab7cd63f25363057c6 ?

  3. sipa commented at 1:21 AM on January 27, 2013: member

    @luke-jr seems I completely forgot about that. Shouldn't be hard to integrate rejection reasons into this, though.

  4. BitcoinPullTester commented at 1:46 AM on January 27, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/a54f890b0ece69a2174c02ac8b877011595e93fc for binaries and test log.

  5. sipa commented at 2:46 PM on January 28, 2013: member

    This closes #2206

  6. Diapolo commented at 3:05 PM on January 28, 2013: none

    Should also address #2146

  7. gavinandresen commented at 6:34 PM on January 28, 2013: contributor

    ACK

  8. sipa commented at 12:49 AM on January 29, 2013: member

    I added a commit to deal with LevelDB errors. It seems to work, but I don't like it. The exception is being caught in several places inside main to do be able to do a graceful shutdown, but it's unclear if all cases are covered (though the worst case is a lesser clean shutdown...). The reported error on stderr can end up being "Error: Error: system error: Database corrupted", followed by shutdown. I guess some hint to try restarting with -reindex should be given. The GUI version relies on ThreadSafeMessageBox, and before the UI is loaded, this simply doesn't do anything, and the program exits without any hint. @laanwj any way to improve that?

    All in all, this will indeed catch errors and cause a shutdown, but it's ugly code and ugly reporting.

  9. BitcoinPullTester commented at 1:08 AM on January 29, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/b054775da97a5db0f1cebd3fc8baf92f280a87d3 for binaries and test log.

  10. gavinandresen commented at 4:23 PM on January 29, 2013: contributor

    ACK if you also fix this bug in init.cpp (otherwise I get a core dump starting with a corrupted database): https://gist.github.com/4665513

  11. CValidationState framework ef3988ca36
  12. Treat coinbase value violation as DoS 86c82bf9d0
  13. Add disk space checks before flushing CCoins cache 18379c8087
  14. Improve dealing with abort conditions 7851033dd6
  15. sipa commented at 3:18 AM on January 30, 2013: member

    Updated.

  16. BitcoinPullTester commented at 3:28 AM on January 30, 2013: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/1e21fd0f8f887740152f2339189aabc679cd983f for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts
    2. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    3. The test suite fails on either Linux i386 or Win32
    4. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)
  17. Deal with LevelDB errors 421218d304
  18. gavinandresen referenced this in commit db3b4ade7b on Jan 30, 2013
  19. gavinandresen merged this on Jan 30, 2013
  20. gavinandresen closed this on Jan 30, 2013

  21. sipa deleted the branch on May 3, 2013
  22. laudney referenced this in commit 37147663ec on Mar 19, 2014
  23. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-18 21:16 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me