Ultraprune + testnet + setgenerate RPC command problem #1946

I'm using an own build containing the new Ultraprune patch on Windows 7. While doing some tests with it, I observed a strange thing.

• start Bitcoin-Qt
• while it syncs the block chain enter RPC console
• enter setgenerate true
• a block was foundm (Client did not yet catch up) and the client now displays Warning: Displayed transactions may not be correct! You may need to upgrade, or other nodes may need to upgrade.

I can't tell if this is a problem of the internal miner (@gmaxwell said it has some errors in) or a problem that raised after Ultraprune though.

The problem persists even after re-starting the client and doing a -rescan.

I was connected only via Tor while testing this, so could it be that my mined block was not broadcasted or via too less nodes (I'm no pro on that part of Bitcoin, sorry if this makes no sense)?

In addition I was now able to reproduce the following error with the same test condition as described above:

Can you try moving the line 'CBlockIndex* pindexPrev = pindexBest;' in CreateNewBlock to inside the large block that is headed by 'LOCK2(cs_main, mempool.cs);' (right below that LOCK2)

Yes, I'll try again with the above applied.

I was still able to get the Warning: Displayed transactions may not be correct! You may need to upgrade, or other nodes may need to upgrade. message, an assertion did not occur anymore ... but I need more time testing this.

Not sure if that message is related to Ultraprune, perhaps I'll take the official 0.7.1 client an fire up the same test scenario.

When connecting a block fails, its work amount is written as bad. When this happens because of an wrong reason, you should restart with a clean datadir, as the warning will remain (until the chain catches up at least).

Actually, you shouldn't be able to mine at all during IBD. I'm sure this bug existed before ultraprune, but the resulting assertion failure is probably new.

@sipa What do you think about adding this into the setgenerate RPC call?

<pre> if (vNodes.empty()) throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Bitcoin is not connected!"); if (IsInitialBlockDownload()) throw JSONRPCError(RPC_CLIENT_IN_INITIAL_DOWNLOAD, "Bitcoin is downloading blocks..."); </pre>

This check is also uses IN the threads itself, but when adding this to the RPC command we prevent spawning 4 threads that just idle and perhaps even prevent block-generation happening in some other weird situations (that currently happen, because I was able to mine a block while beeing on IBD), which causes further problems.

So start the mining threads for RPC call only when connected and not in IBD (explicit usage) and still alow the old behavior, when using -gen (as you most likely want to start mining, when the client is ready here - implicit usage).

I was not able to reproduce the assertion again, so your idea seems to have fixed that one! You think it was a locking problem?

Edit: Do you thing it makes sense to create a pull for your assert fix here or is that obsolete with your last patches to master?