CreateNewBlock: Unit tests and runtime validation check #1246
Conversation
|
This is bugged. ConnectBlock requires the block to be on the disk already :( |
|
In fact, this might even corrupt the blkindex.dat somehow, so if you tried it, I suggest rebuilding with -loadblock :/ |
|
OK, this seems to work now. |
|
Eligius has been running this from block 179513 (56 blocks found) and EclipseMC from 180573 (11 blocks), totalling 67 valid blocks with no problems found. |
|
Now includes tests for CreateNewBlock |
src/main.cpp
Outdated
There was a problem hiding this comment.
Why can't this formula be used in the justcheck case as well?
There was a problem hiding this comment.
Position 1,1,1 is "magic" in FetchInputs to refer to the memory pool. Otherwise, it will try to find it on disk
There was a problem hiding this comment.
Ok, can you add a comment to explain that?
…for proof-of-work and merkletree, since those need to be provided later) This throws an exception from CreateNewBlock otherwise, which is not safe without bitcoin#1245!
|
Added comments per sipa's request, rebased on master, and fixed a bug in diskless BDB I found testing. |
|
getmemorypool's caching mostly makes testing this impossible, but I did some vague performance testing by disabling it (so every call goes to CreateNewBlock): IMO, even the worst-case scenario of 0.00321 seconds per transaction processed is worth the additional safety checking, especially when considering the extra caching getmemorypool and getwork both do to minimize CreateNewBlock calls. |
|
ACK (together with #1245). I really like having unit tests that test blockchains. |
|
ACK |
|
Rebased. #1245 did not need rebasing. |
There was a problem hiding this comment.
This is a very un-C++-ish way of doing this. The C++ Way would be an abstract base CDBEnv class, with concrete subclasses for unit testing (CMockDbEnv or CMemoryDbEnv) and records-to-disk.
But the use of the global bitdb variable is also very un-c++-ish, and if Mike Hearn's experiments with LevelDB and Pieter's experiments with a new wallet storage format go well this whole subsystem might get junked anyway....
Conflicts: src/main.cpp
|
Does not compile 32 bit: test/miner_tests.cpp:89: error: integer constant is too large for ‘long’ type |
|
Explicitly made the literals long long. Apparently "long long" was not a standard type until C++11, and compilers implementing it as an extension did not support mere numeric literals bigger than long (the C++11 standard requires these literals be handled properly). GCC 4.5 works with both ways. In any case, being explicit doesn't seem like it hurts, so I've added that. Let me know if it still doesn't work. |
4 participants
CreateNewBlock: Check that the produced CBlock is acceptable (except for proof-of-work and merkletree, since those need to be provided later)
This throws an exception from CreateNewBlock otherwise, which is not safe without #1245!