Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add block blacklisting RPC #2839

Closed
wants to merge 2 commits into from
Closed

Add block blacklisting RPC #2839

wants to merge 2 commits into from

Conversation

sipa
Copy link
Member

@sipa sipa commented Jul 20, 2013

This is a rebased version of a patch that mining.bitcoin.cz used during the march 11 2013 hardfork, to be able to continue using 0.8 while still mining the 0.7 chain.

The reason for submitting it to mainline is:

  • When implementing this, I found that there were a few edge-cases in the reorganization handling, which are fixed here. They probably won't ever occur in normal operation, but I prefer the code to be robust.
  • For emergencies, having a blacklistblock RPC is certainly useful to have in the code, though I prefer not having it in normal releases. It's only enabled when compiling with ENABLE_BLOCK_BLACKLISTING. The RPC code is always compiled, so we can catch refactorings that would break it, though - just the index entry is not present normally.

sipa added 2 commits July 20, 2013 13:13
This fixes some weird reorganization issues that probably don't normally
occur, but can be triggered when blacklisting arbitrary blocks is enabled.
As this is a fairly dangerous operation, it is normally disabled when
building. Define ENABLE_BLOCK_BLACKLISTING to enable compiling it in.
@BitcoinPullTester
Copy link

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/9fe9d788a9eaf40cc5dd1171807b24c9bd170104 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.

@gmaxwell
Copy link
Contributor

Could also be used for whitebox test instrumentation: E.g. have a table with the proper utxo state for every height, and use blacklisting to walk all the way back while checking it.

@jgarzik
Copy link
Contributor

jgarzik commented Jul 22, 2013

ACK

@mikehearn
Copy link
Contributor

I was thinking it'd be better to allow tx hash blacklisting. To blacklist a block you'd blacklist its coinbase.

TX blacklisting would allow app developers to more easily test double spend and reorg handling in regtest mode.

@sipa
Copy link
Member Author

sipa commented Jul 30, 2013

@mikehearn Makes sense, but that'd be more work, as there is no transaction index (and the optional one isn't used for validation). For blocks, there is already a mechanism for marking them invalid in the block index db.

@mikehearn
Copy link
Contributor

Bitcoin already re-validates the last N hundred blocks on startup, right? Do you imagine your patch ever being used to blacklist a block more than a few hundred blocks deep? If not, then it would still seem to be possible.

@sipa
Copy link
Member Author

sipa commented Aug 1, 2013

Not sure how that is related. This is not something that happens at startup - it can mark a block invalid during execution, and it will reorganize away from it instantly. If anything, this is very useful to test edge cases in the block connection logic.

For blacklisting transactions that are in the blockchain already, you'd need a transaction index anyway, and if you do, it's easy enough to look up the block your transaction is in, and blacklist that. For blacklisting mempool transactions (which is more useful, I think - it's unlikely that you hate a transaction that much that you want to cause a hardfork over it), a different approach is needed, but it'd very simple to do: just call mempool.erase from an RPC call.

@gavinandresen
Copy link
Contributor

Just to clarify: blacklistblock permanently blacklists a block; the only way to undo it is to -reindex the block chain.

I think that should go in the blacklistblock help message; people might assume that the blacklist state is memory-only and can be reset by restarting.

Otherwise: ACK.

@yhaenggi
Copy link

yhaenggi commented Aug 7, 2013

ACK (even it isnt up to me), this could save us alot of time if we run in the same trouble again

@wtogami
Copy link
Contributor

wtogami commented Aug 27, 2013

It sounds like people are in favor of this but only want the help message to more explicitly explain what it does and the danger of misusing it?

@sipa
Copy link
Member Author

sipa commented Sep 23, 2013

@mikehearn I wrote this pull request as preparation for headers-first sync, and the current headers-first sync pull request still includes it, but most of the code touched here (except the actual RPC implementations) is rewritten for headers-first anyway.

@mikehearn
Copy link
Contributor

Maybe it should be closed then.

@wtogami
Copy link
Contributor

wtogami commented Oct 14, 2013

Close?

@laanwj
Copy link
Member

laanwj commented Nov 22, 2013

So should this be closed?

@sipa
Copy link
Member Author

sipa commented Nov 22, 2013

Closing until updated.

@sipa sipa closed this Nov 22, 2013
IntegralTeam pushed a commit to IntegralTeam/bitcoin that referenced this pull request Jun 4, 2019
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants