Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling during validation #2224

Merged
merged 5 commits into from Jan 30, 2013

Conversation

@sipa
Copy link
Member

sipa commented Jan 27, 2013

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).

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Jan 27, 2013

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

@sipa

This comment has been minimized.

Copy link
Member Author

sipa commented Jan 27, 2013

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

@BitcoinPullTester

This comment has been minimized.

Copy link

BitcoinPullTester commented Jan 27, 2013

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

@sipa

This comment has been minimized.

Copy link
Member Author

sipa commented Jan 28, 2013

This closes #2206

@Diapolo

This comment has been minimized.

Copy link

Diapolo commented Jan 28, 2013

Should also address #2146

@gavinandresen

This comment has been minimized.

Copy link
Contributor

gavinandresen commented Jan 28, 2013

ACK

@sipa

This comment has been minimized.

Copy link
Member Author

sipa commented Jan 29, 2013

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.

@BitcoinPullTester

This comment has been minimized.

Copy link

BitcoinPullTester commented Jan 29, 2013

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

@gavinandresen

This comment has been minimized.

Copy link
Contributor

gavinandresen commented Jan 29, 2013

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

@sipa

This comment has been minimized.

Copy link
Member Author

sipa commented Jan 30, 2013

Updated.

@BitcoinPullTester

This comment has been minimized.

Copy link

BitcoinPullTester commented Jan 30, 2013

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)
gavinandresen added a commit that referenced this pull request Jan 30, 2013
Improve error handling during validation
@gavinandresen gavinandresen merged commit db3b4ad into bitcoin:master Jan 30, 2013
@sipa sipa deleted the sipa:valstate branch May 3, 2013
laudney pushed a commit to reddcoin-project/reddcoin that referenced this pull request Mar 19, 2014
Improve error handling during validation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.