bitcoin/bitcoin — pull request #4688 by SergioDemianLerner (2014-08-12 19:56:57+00:00)
Title: Improvement to the Test Framework in the processing of test blocks

For several reasons the current test framework does not allow to easily incorporate new unit tests that append specially crafted blocks to the blockchain using ProcessBlock().
This was pointed out by Mike Hearn on the development list in the thread with subject "[Bitcoin-development] Question on creating test cases for block.CheckBlock()" (http://article.gmane.org/gmane.comp.bitcoin.devel/5939).

After debugging the Bitcoin core, we found that this is because of three reasons:

1. The miner_tests.cpp leaves the transaction pool in an invalid state and assumes the blockchain is empty on start. So other independent unit tests cannot be run before nor after (without proper cleaning) if they intend to use CreateNewBlock(), which collects transactions from the pool.

2. Creating test blocks requires proof-of-work. Even if the starting difficulty is low (1) the required proof-of-work is still too high to allow for the dynamic creation of test blocks. Instead, unit test have to be "pre-mined". This makes the code more opaque and increases the effort of changing the code.

3. Only the first unit test can start with an empty blockchain, all subsequent tests have to start with the blockchain in the state where the previous test left it. This has the following problematic consequences:
a. Each unit test makes the blockchain longer. This breaks the test sequence when a checkpoint is reached because a checkpoint requires a pre-determined hash digest. Moreover, there are exception cases for certain block heights (e.g. regarding allowing two transactions with the same hash) which could be violated.
b. Certain combinations of unit tests are inherently impossible to implement in a single block-chain if not run in an specific order.
Other combinations may cause unexpected consequences. For example, since version 1 blocks do not have the height field included in the coinbase field of the generation transaction, a unit test may create a coinbase tx with a future height in the height field and prevent a coinbase tx with the same hash to be used afterward when only blocks v2 are accepted (this happened to us while testing).
c. The standard unit testing policy that unit tests should be not depend on each other's output is violated.
This makes debugging more difficult.
d. "Pre-mining" unit tests is impossible unless all previous unit tests are known and never change. (However, we are proposing to eliminate the proof-of-work check regardless.)

We believe that testing the block acceptance rules is crucial for the safety of the application and so we wrote this patch.
By restarting the blockchain before every unit test that requires testing block acceptance we have the guarantee that all tests are independent, executed in a predefined reproducable environment, and don't unintentionally hit checkpoints or other exceptions. Nevertheless, each unit test decides whether to re-use or reset the block-chain. We haven't perceived any significant delay while performing the destruction and creation of the block-chain during the execution of the test application. This is because file space allocation functions are fast on modern filesystems. Nevertheless, UNDOFILE_CHUNK_SIZE/BLOCKFILE_CHUNK_SIZE can be reduced during test case execution if the block-chain is reset many times.

In detail, this patch solves 1.,2.,3. from above by:

- Providing a method to reset the blockchain to the starting state (testingSetupManager.SetupGenesisBlockChain())
- Allowing to dynamically skip the proof-of-work testing (supressCheckBlockWork = true)
- Fixing the bug in miner_tests.cpp which leaves in the memory pool invalid transactions (mempool.clear() missing).

We've also found the exact procedure that can be used to programmatically destroy and re-create the blockchain correctly, which was not implemented and nor documented. Some cleanup methods existed but some other were added because they were missing.

This could be of great help to re-create completely the blockchain in case a severe damage has been detected, without restarting the application.

As a bonus, 7 unit tests have been added:
- ToCheckBlockUpgradeMajority (untested before)
- EnforceBlockUpgradeMajority (untested before)
- RejectBlockOutdatedMajority (untested before)
- "bad-cb-height"
- "bad-version"
- "time-too-old"
- "bad-txns-nonfinal"

Last, we added a way to leave the blockchain unaltered after the test suite is over to debug the unit tests themselves (testingSetupManager.keepTestEvidence = true)

Note: The BerkeleyDB environment field was converted into a heap allocated object because BerkeleyDB handles are not meant to be re-used after close, and the block-chain environment can be closed and re-opened in the unit tests. This is explained in http://docs.oracle.com/cd/E17275_01/html/api_reference/CXX/envclose.html as
"After DbEnv::close() has been called, regardless of its return, the Berkeley DB environment handle may not be accessed again."

Sergio Demian Lerner & Timo Hanke