I needed this to test the new "reject" p2p message, but it should be generally useful for fuzz-testing network message handling code.
Note: you'll quickly trigger the DoS-and-ban code if you connect to random peers, this is most useful in a two-nodes-on-your-development-machine testnet-in-a-box or -regtest setup.
I agree that this is useful. However I'm not sure whether we should include test/fuzzing/stresstest code in the core.
If we want this we should put it in #ifdefs, add a configure option to enable it and not ship it in gitian builds (IMO).
I tend to agree, I remember I wanted to include an GCC stack smashing protection test via a switch, which seemed not like a good idea in the end. A guard via #ifdef ALLOW_FUZZTEST and a compiler option would be good IMO.
make it only fuzz localhost peers? :)
ACK
Lets not make barriers to this kind of testing like having to compile it in; implementations should be robust enough to handle junk getting sent at them from the occasional buggy peer. If they aren't that robust, they need to be improved frankly.
edit: been running this for a few hours, and it looks like Bitcoin itself can be crashed due to the fuzz-tester...
442 | @@ -440,6 +443,8 @@ class CNode 443 | AbortMessage(); 444 | return; 445 | } 446 | + if (mapArgs.count("-fuzzmessagestest")) 447 | + Fuzz(atoi(mapArgs["-fuzzmessagestest"]));
Fuzz(GetArg("-fuzzmessagestest", 0));?
ACK.. no objection to it being a runtime feature.
At least keep it undocumented in -help then. There is already such a huge list of command line options, let's not add options that are not useful for 99.9% users and will cause them to be banned if they add it by accident.
Added a comment to make it clear the -*messagestest options are not documented on purpose, since they are for implementors, not end-users.
I do think this should be a built-in run-time feature, it should be very helpful to re-implementors who are likely to be programming in a different language and might not want to bother setting up a dev environment to build themselves.
Tweaked with @Diapolo's suggestion to use GetArg instead of atoi (made it default to 1-of-10 messages fuzzed because that seems about right). @petertodd: please file a bug or send an email to bitcoin-security@lists.sourceforge.net if you found a bug or a vulnerability.
Changed CDataStream::GetAndClear() to use the most obvious
get get and clear instead of a tricky swap().
Added a unit test for CDataStream insert/erase/GetAndClear.
Note: GetAndClear() is not performance critical, it is used only
by the send-a-message-to-the-network code. Bug was not noticed
before now because the send-a-message code never erased from the
stream.
I needed this to test the new "reject" p2p message, but it should be generally
useful for fuzz-testing network message handling code.
Rebased; fixed a crashing (assertion failed) bug caused by the fuzz-tester deleting at the beginning of the CDataStream (normal bitcoin usage never does that).
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/9038b18f4655a5b8ad119d768decd1c693ebd7dd 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.
@gavinandresen Been running this for about a day with the crash fix and -fuzzmessagestest=100 No problems found.