Add 'blacklistblock' and 'reconsiderblock' RPC commands.

[sipa]

These can be used for testing reorganizations or for manual intervention in case of chain forks.

[sipa]

Tested on testnet by issuing blacklistblock of an older block during IBD, and later calling reconsiderblock with the same hash. Also the same with with a restart of the node in between.

[sipa]

Heh, I can even successfully mark the genesis block invalid and recover it seems.

[gmaxwell]

\O/ @TheBlueMatt Any interest in making pulltester use this? in particular, the reconsideration path is obviously completely untested right now. Any ideas how we can test that?

[sipa]

@gmaxwell It would be 'ugly' for comparison tool (which is a blackbox network behaviour tester) to invoke RPCs that are deliberately designed to override the correct network behaviour.

A python-based RPC test would be very nice for this, which mines some blocks, blacklists, mines more, reconsiders, and checks whether it recovers. I'll have to dig into that framework code, though.

[gmaxwell]

@sipa the case thats hard to test is we'd like to have a invalid block get rejected through the normal pathways and then get reconsidered.

[gmaxwell]

I'm currently trying to reorg the main chain on a node with a wallet back down to height 1 and it's taking about 7 seconds per block. Makes it a little frustrating for testing.

[gmaxwell]

Minor comment, we might want to make a help section for 'diagnostic/testin' commands and move these and mocktime and maybe the txoutset commands into it.

[gmaxwell]

After several kill/restart cycles while it was reorging back down to block 1 (but thousands of blocks away) one time when it came up it didn't continue going on its own and I had to resend the blacklist command. Any idea why?

[sipa]

@gmaxwell logs?

[gmaxwell]

What it seems to actually be doing on restart is only reorginizing some finite number of blocks and then stopping and sitting stuck until I perform the RPC again.

E.g.

2014-11-20 01:29:15 Rescanning last 36 blocks (from block 330629)...
2014-11-20 01:29:35 rescan 20103ms
2014-11-20 01:31:55 UpdateTip: new best=00000000000000001585818b3733f687ff1263fd822dd22da13173f86a40d862 height=330664 log2_work=81.501065 tx=51787936
2014-11-20 01:33:18 UpdateTip: new best=000000000000000010f9ba685d8ab2ed22418cc8987eeccf51814dee6cbd4dac height=330663 log2_work=81.500992 tx=51787143
2014-11-20 01:33:21 ERROR: AcceptToMemoryPool : nonstandard transaction: tx-size
[...]
2014-11-20 05:55:46 UpdateTip: new best=00000000000000001a39a44955d2a76a1a88c4ed09f6e132298df0da5ff57fb8 height=330486 log2_work=81.488191 tx=51689100
2014-11-20 05:57:26 UpdateTip: new best=000000000000000016ae5f443314da5818dad541884e1fb8c9dc311992a51842 height=330485 log2_work=81.488119 tx=51688536
2014-11-20 05:57:26 mapBlockIndex.size() = 331107
2014-11-20 05:57:26 nBestHeight = 330485
2014-11-20 05:57:26 setKeyPool.size() = 101
[...]
2014-11-20 06:12:18 ERROR: AcceptBlockHeader : block is marked invalid
2014-11-20 06:12:18 ERROR: invalid header received
2014-11-20 06:12:18 ProcessMessage(headers, 26085 bytes) FAILED peer=5

(very poor performance because I'm running in valgrind; though it's pretty slow without the valgrind)

(also, if you care to reproduce, you'll need to workaround the bug w/ CheckForkWarningConditions to be called from ActivateBestChainStep with pindexBestForkBase null, and we happily dereference it. ... I think this existed prior to this patch, Matt tells me we previously had a PR relevant to it but he couldn't figure out how to trigger it then. To me it looks triggerable in the current code, absent your patch, simply by getting a block marked invalid and then shutting down and allowing the reorg to happen without the invalid block ever being hit in the current execution run).

[sipa]

Oh, that's expected. You need to wait for the RPC to finish I'm afraid.

The current ActivateBestChain assumes the currently active chain is always valid, and changing that assumption means that more things have to change, which I'd rather not do now (the patch is very clean, as there is no code changed except code invoked by those RPCs themselves).

[gmaxwell]

@sipa Seems reasonable enough. My host (with the checkforkwarning fix) is still grinding away using this reorging its chain.

[gmaxwell]

15:05 < gmaxwell> sipa: so I realize I keep thinking of your blockblackist thing performing the verb "invalidate" not "blacklist". Perhaps the rpc should be
renamed? e.g. invalidateblock / reconsiderblock ?
15:05 < sipa> gmaxwell: fair enough

[gmaxwell]

ACK

[jgarzik]

Any chance of batching those writes?

[sipa]

@jgarzik I'll be happy to turn them into batch writes after #5241 is in; now it would just be duplicate work (and there's no real requirement for these to be actually fast, as they are for very manual emergency intervention and testing only).

[rebroad]

What's the reason for this functionality?

[sipa]

A much earlier version of this patch was actually used by slush's pool in march 2013 when the 0.7 vs 0.8 split happened, to force their 0.8 node to switch to the 0.7 chain. Having such functionality present is definitely useful for manual interventions.

Other than that, testing very-large scale reorganizations for example.

[sipa]

Maybe the invalidateblock RPC needs a big fat warning when used on mainnet, though.

[gmaxwell]

Or just hide it in the RPC help if you think users are too likely to foot-gun by forcing themselves off the best chain.

[gmaxwell]

ACK. Looks and tests good, though a bit slow.

Wumpus suggested that perhaps the rpc be disabled unless some -blockdebugging=1 was set to reduce the footgun risk. If you wanted to do that I'd find it agreeable too.

[laanwj]

Ye, please put this behind an option for Advanced Usage and don't display them in the help by default.

[sipa]

@laanwj @gmaxwell Moved them to a 'hidden' category.

[laanwj]

If everyone else agrees that moving them to a hidden category is enough, I'm fine with that.

This only adds code so there is effectively no regression risk. utACK for 0.10.

commithash bd9aebf https://dev.visucore.com/bitcoin/acks/5316

[sipa]

I dislike having them only being available behind an command-line option, as there may be benefit in case of emergencies to not having to restart nodes first.

[gmaxwell]

I'm still an ACK, and I've tested this a fair bit more... it's pretty fun zapping blocks and watching it move up and down the chain to get back to the new best position.

[laanwj]

Merged w/ added commit f86a24b that moves setmocktime to the hidden category as well.