Remove side effect in assertion in ProcessGetData #4287

A side-effect was introduced into an assertion in 7a0e84d. This commit fixes that.

ACK -- as a first step.

It is disappointing that we handle disk errors in this way. Surely we need to at least flush the wallet, before crashing??

Agree, but that should be solved by a better assert function that shows a message and calls StartShutdown() instead of aborting immediately (with special handling if already in shutdown sequence...). Doing anything else for this special case would leave the same problem in the 100's of other places where assert is used.

Note that this is the only example of a ReadBlockFromDisk() call that fails so abruptly. All the others handle failure more gracefully.

Sure, the error could be handled differently, for example by disconnecting the node.

I think if you cannot read a block from disk, Something Major Is Wrong, and shutting down the node ASAP is the right thing to do. Both for this case, and any other case where we detect a fault in already stored data. ie. Data that was previously hashed, verified, stored... and now fails checksum or disk read.

Well that brings us back to my suggestion above about a better assert...

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4a48a0671d2e02ba2a86af98282a452f23546b9e for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.