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
Headers-based synchronization and parallel block download #2964
Conversation
Pulltester where are you? |
Dang. @TheBlueMatt: how does pulltester send/announce blocks, does it answer getheaders, and how does it know a block was received? |
Currently block-tester doesnt support getheaders requests, so that needs fixed |
@sipa fyi, now you can easily run the same scripts as pull-tester to check locally, in case you're looking into fixing up the block-tester. Just add --with-comparison-tool to your configure, and this test will be enabled during 'make check':
If you replace the .jar, just 'make check' again. |
Rebased, though I haven't investigated the pulltester failure further. I assume it has to do with actually changed semantics, as the best known not-known-to-be-invalid header chain is always considered the main chain, and pindexBest is always within this chain. As the decision is not just made per-received-block, you could have a valid chain A-B-C, and another chain A-B-C'-D, where D is invalid, in which case C' is reported as best chain instead of C (assuming they have the same amount of work, so the choice should be just as good). Reverting to the old behaviour here would require a hack like reducing the scoring of blocks that have invalid successors for example, but that seems pointless to me, and there are likely other differences as well. I suppose that during reorganizations, pulltester should just accept any not-invalid block chain with maximum PoW as correct? |
@TheBlueMatt Any help decoding pulltester's output? |
Have you tried the latest comparison tool jar from https://github.com/TheBlueMatt/test-scripts ? |
@TheBlueMatt (as discussed on IRC) Yes, this gets us further already, but it seems the getheaders implementation in the comparison tool is too incomplete to do a successful multi-block reorg with headers-first. So this means fixing that becomes sort of a blocker for this; unfortunately, I don't have the time any time soon to look into that. |
SendMessages() tries to acquire a cs_main lock now, but this isn't nessecary for much of its functionality. Move those parts out of the locked section, so they can always be performed, and we hold cs_main for a shorter time.
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.
Instead of using "getblocks" to learn about the hashes of blocks peers have, use "getheaders" to retrieve the headers of those blocks entirely. This allows the client to immediately build an internal representation of the block tree. This means the best chain (or at least its most likely candidate, disregarding invalid transactions) is almost instantly known, and block download can proceed as a secondary (though in practice simultaneous) and very robust process to acquire the transaction data along this already-known best chain. Advantages include: * No more vulnerabilities related to orphan blocks, as they simply don't exist anymore. We only fetch blocks along the best chain; any other block we're not interested in. * No more need for checkpoints to determine bounds on difficulties of received blocks, as we never receive a block without knowing (the headers of) its ancestry. For a new, smaller vulnerability (sending many long low-difficulty header chains, consuming memory), checkpoints are still useful. * No more need for checkpoints for safety when disabling signature check. A rule like "Do not check signatures in blocks covered by N weeks worth of hash power" could be used, though this is currently not yet implemented. * The IsInitialBlockDownload() become very precises and safe: we're syncing a backlog as long as there are headers in our best chain for which blocks are missing. There is no need for relying on peers' reported number of blocks anymore. * It can serve as a basis to implement SPV mode, which is basically the same logic, but with the blockchain-driven block fetching replaced by wallet-driven filtered block fetching. * Efficient parallel block download, and generally avoiding many edge cases that were possible during the previous getblocks-based synchronization (including frequent duplicate downloads, orphans and and needing to wait for the next block to be announced before resuming). Additionally, good synchronization speed does not rely on guessing a good initial peer to fetch from anymore - a single slow peer does not hurt you anymore. This also means that throttling upload speeds will no longer be a problem to the network. For now, block sync is only done from outbound peers, to retain the "no listening == less bandwidth" property that currently exist. As soon as enough nodes upgrade to headers-first sync, this can be removed. Potentially significant semantical changes: * In case a new best chain is known, but its tip blocks are not yet known, the best known ancestor of the new tip is used instead of the old tip. On the other hand, during this period it is known that we're not up to date. The effects of this on mining should be considered, probably. More in detail, changes include: * All redundant globals are gone (nBestHeight, nBestChainWork, nBestInvalidWork, hashBestChain) - instead, we just have 4 pointers into the block tree (pindexGenesisBlock, pindexBest, pindexBestHeader, pindexBestInvalid). * ProcessBlock and AcceptBlock are split into ProcessBlockHeader and AcceptBlockHeader. * Instead of 'getblocks', push out 'getheaders' to the sync node. * Implement 'headers' P2P handler, causing headers to be included in the block index. * pindexBest remains as pointer to the "current state of the UTXO set" (chainstate), meaning the point until where we are synchronized. pindexBestHeader is added as a pointer to the tip of the currently best known (and validated, except for transaction validity) header. pindexBest is guaranteed to be always an ancestor of (or equal to) pindexBestHeader. * A new function SwitchToBestHeader finds the best header in the database, and switches to it. SwitchToBestBlock connects blocks along this best chain. These replace ConnectBestBlock. * The block tree internally can deal with orphan blocks (whose parents are unknown), but this mechanism is only used during reindexing, as block may be stored out-of-order. A new function LinkOrphans connects orphans to the tree, and replaces some code in LoadBlockIndex. * Actual blocks are only fetched along the known-best chain, using parallel block downloading logic in SendMessages. For each peer, a (bounded) list of requested blocks is maintained. New requests are pipelined, as the requested blocks arrive. As the tree they belong to is already known, we can receive and store them out-of-order. This also means that the concept of orphan blocks disappears. If one peer stalls us, it is disconnected and the blocks requested from it are sent elsewhere. This is nicer both for us and for them. * One exception to the headers-first syncing, is when we are fully synced (pindexBest==pindexBestHeader), and a new block 'inv' is announced. In this case, the block is fetched immediately ('getdata') after asking for the headers preceding it, to avoid the extra roundtrip. * SetBestChain (which handled reorganizations) is no longer necessary, as SwitchToBestHeader and SwitchToBestBlock only ever connect/disconnect individual blocks. It is therefore replaced by two simpler functions, ConnectTip and DisconnectTip, implented using helpers WriteChainState and UpdateTip. * In several places code is added to deal with the case that a CBlockIndex entry does not have corresponding block data available.
Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/bd49d084540ebd15c19b3dcb660802aa40da5f2d for test log. This pull does not merge cleanly onto current master |
Closing. This is outdated, as I've been reworking this as more managable patches/pull requests. |
The blockchain takes weeks to download. Please merge this in ASAP. This is the absolutely most important thing right now. |
@haltingstate Then help reviewing and testing #3884, which is a step towards this. |
(see individual commits, in particular "Switch to headers-based synchronization", for detailed explanation of the changes).
This implements headers-based synchronization and parallel block download. Summary of the changes:
getheaders
instead ofgetblocks
based synchronization, so that the best block in the chain is always known before actually downloading it. No more orphan blocks, ever.getblockheader
RPC command.