Headers-first synchronization #4468

pull sipa wants to merge 11 commits into bitcoin:master from sipa:headersfirst8 changing 15 files +480 −502
  1. sipa commented at 11:09 pm on July 5, 2014: member

    Here’s a first (well, second) version of a fully-functional headers-first synchronization.

    Many changes:

    • Do not use ‘getblocks’, but ‘getheaders’, and use it to build a headers tree.
    • Blocks are fetched in parallel from all available outbound peers, using a limited moving window. When one peer stalls the movement of the window, it is disconnected.
    • No more orphan blocks. At all. We only ever request a block for which we have verified the headers, and store it to disk immediately. This means that a disk-fill attack would require PoW.
    • Some new fields in getpeerinfo:
      • ‘syncheight’: the height up to which we’ve validated this peer’s headers.
      • ‘commonheight’: the height up to which we have all blocks in common with this peer.
      • ‘inflight’: the heights we’re currently requesting from this peer.
    • Require protocol version 31800 for every peer (released in december 2010).
    • No more syncnode (we sync from everyone we can, though limited to 1 during initial headers sync).
    • Introduce some extra named constants and comments.
    • Reindexing support for out-of-order blocks on disk.
  2. gmaxwell commented at 4:34 am on July 6, 2014: contributor

    While syncing with this code from a pair of local peers:

    2014-07-06 04:26:28 UpdateTip: new best=000000001ee1d3053357a374d6d9746e80d56a666f3827c92e80e0d1b3f2f2a1 height=1888 log2_work=42.883429 tx=1917 date=2009-01-26 03: 21:12 progress=0.000023 2014-07-06 04:26:28 nActualTimespan = 942027 before bounds 2014-07-06 04:26:28 GetNextWorkRequired RETARGET 2014-07-06 04:26:28 Params().TargetTimespan() = 1209600 nActualTimespan = 942027 2014-07-06 04:26:28 Before: 1a016164 0000000000000161640000000000000000000000000000000000000000000000 2014-07-06 04:26:28 After: 1a011337 000000000000011337c4f5c28f5c28f5c28f5c28f5c28f5c28f5c28f5c28f5c2 2014-07-06 04:26:28 ERROR: AcceptBlock() : prev block not found 2014-07-06 04:26:28 ERROR: ProcessBlock() : AcceptBlock FAILED 2014-07-06 04:26:28 Misbehaving: 192.168.42.76 (0 -> 10) 2014-07-06 04:26:28 UpdateTip: new best=00000000bf3fc4c4ab6737df907f613b5aa86373d426b5e86a1009030852f129 height=1889 log2_work=42.884193 tx=1918 date=2009-01-26 03:26:22 progress=0.000023

    (not the Misbehaving)

    Also, it seems to only be pulling from one so far:

     0{
 1    "addr" : "192.168.42.76",
 2    "services" : "0000000000000001",
 3    "lastsend" : 1404621016,
 4    "lastrecv" : 1404621016,
 5    "bytessent" : 7131512,
 6    "bytesrecv" : 131870355,
 7    "conntime" : 1404620769,
 8    "pingtime" : 0.07961100,
 9    "version" : 70002,
10    "subver" : "/Satoshi:0.9.99/",
11    "inbound" : false,
12    "startingheight" : 309423,
13    "banscore" : 10,
14    "syncheight" : 309424,
15    "commonheight" : 113772,
16    "inflight" : [
17        113773,
18        113774,
19        113775,
20        113776,
21        113777,
22        113778,
23        113779,
24        113780,
25        113781,
26        113782,
27        113783,
28        113784,
29        113785,
30        113786,
31        113787,
32        113788
33    ]
34},
35{
36    "addr" : "192.168.42.87",
37    "services" : "0000000000000001",
38    "lastsend" : 1404621015,
39    "lastrecv" : 1404621009,
40    "bytessent" : 1442,
41    "bytesrecv" : 2342,
42    "conntime" : 1404620769,
43    "pingtime" : 0.04079200,
44    "version" : 70002,
45    "subver" : "/Satoshi:0.9.99/",
46    "inbound" : false,
47    "startingheight" : 309423,
48    "banscore" : 0,
49    "syncheight" : -1,
50    "commonheight" : -1,
51    "inflight" : [
52    ]
53}

    It started pulling from it eventually, perhaps when a block came in?

  3. gmaxwell commented at 5:13 am on July 6, 2014: contributor

    2014-07-06 04:38:07 Leaving block file 0: CBlockFileInfo(blocks=119950, size=134216038, heights=0…309426, time=2009-01-03…2014-07-06) 2014-07-06 04:38:37 Leaving block file 1: CBlockFileInfo(blocks=11284, size=134206314, heights=119942…131232, time=2011-04-24…2011-06-16)

    The first range looks wrong. :)

  4. sipa commented at 7:46 am on July 6, 2014: member
    @gmaxwell The ‘prev block not found’ should be fixed; there is a code path to fetch blocks directly (ignoring the headers-based fetching), in case we’re very close to being synced (to avoid an extra roundtrip for newly inv’ed blocks)… but the time comparison used < instead of >.
  5. gmaxwell commented at 1:45 pm on July 6, 2014: contributor
    3hr 46 minute resync over the network here. I’m a bit confused that the ping time to the two peers I was pulling from was still >10 seconds even when the sync was well into the cpu bound part (for their own part, the peers were fairly low on cpu utilization).
  6. sipa commented at 3:57 pm on July 6, 2014: member
    4h21m here, from random peers, and default dbcache.
  7. laanwj commented at 5:04 am on July 7, 2014: member
    Woohoo! Testing…
  8. in src/main.cpp: in eb7822c91b outdated 
    292         CNodeState *state = State(itInFlight->second.first);
293         state->vBlocksInFlight.erase(itInFlight->second.second);
294         state->nBlocksInFlight--;
295-        if (itInFlight->second.first == nodeFrom)
296-            state->nLastBlockReceive = GetTimeMicros();
297+        state->nStallingSince = 0;
    rebroad commented at 5:30 am on July 7, 2014:
    Why wait until a complete block is received before marking it as not stalling? Wouldn’t it be better to consider it as not stalling as long as a block is being downloaded (even if it takes a while on a slow connection)?
    sipa commented at 12:54 pm on July 7, 2014:

    “Stalling” is already a rather strong condition: it means all blocks in flight are from a single peer, and we can’t ask any peer for other blocks because they are outside the download window. It does not just mean that we’re not receiving anything from this peer, it means we can’t download anything from anyone because of this peer. The 2 second delay before actually connecting is to give us some chance to act on blocks that were already received while we were busy.

    That said, this is certainly not perfect. We’ll need some tracking of peers’ speed and latency to adjust window sizes, for example. I really just want something that reasonably well in now.

  9. in src/main.cpp: in eb7822c91b outdated 
    325     // Make sure it's not listed somewhere already.
326     MarkBlockAsReceived(hash);
327 
328-    QueuedBlock newentry = {hash, GetTimeMicros(), state->nBlocksInFlight};
329-    if (state->nBlocksInFlight == 0)
330-        state->nLastBlockReceive = newentry.nTime; // Reset when a first request is sent.
    rebroad commented at 5:32 am on July 7, 2014:
    Are you trying to break #4431 ?!
    fanquake commented at 6:00 am on July 7, 2014:
    Sipa already mentioned that headers first would render #4431 almost obselete
    rebroad commented at 11:47 am on July 7, 2014:
    yes he did, but it’s somewhat annoying when so newly introduced variables are removed by the same author in subsequent pulls.
    laanwj commented at 12:28 pm on July 7, 2014:
    The node / block chain handling code is very much in flux at the moment, you’ll have to live with that. I can understand annoyance if it is just some white-space reordering that broke your pull for the zillionth time, but these are difficult and important changes and it’s not possible to know everything in advance.
    sipa commented at 12:31 pm on July 7, 2014:
    I understand, but some temporary code was necessary to get the block tracking code into the existing (non headers first) mechanism. I think I mentioned that in the PR or commit that introduced it.
  10. in src/main.cpp: in eb7822c91b outdated 
    2684-    pnode->PushMessage("getblocks", chainActive.GetLocator(pindexBegin), hashEnd);
2685-}
2686-
2687 bool ProcessBlock(CValidationState &state, CNode* pfrom, CBlock* pblock, CDiskBlockPos *dbp)
2688 {
2689-    // Check for duplicate
    rebroad commented at 5:44 am on July 7, 2014:
    If duplicate blocks aren’t requested, then surely if we receive them this should be considered misbehaviour.
    sipa commented at 12:57 pm on July 7, 2014:
    Good catch. I can’t check the code now, but if there is no other logic for punishing duplicates, that will need fixing.
    sipa commented at 3:03 pm on July 8, 2014:
    Fixed. Made it a level 20 dossable offense (in AcceptBlock, which already has cs_main).
    gmaxwell commented at 4:25 am on September 4, 2014:
    oh no, you can’t currently punish blocks you didn’t request, because when a node finds a block itself, it relays it without INVing. (IIRC)
    sipa commented at 8:39 am on September 4, 2014:
    That only happens for just-mined blocks - peers shouldn’t ever have those already?
    rebroad commented at 8:46 am on September 4, 2014:
    hold on… it won’t be possible to receive a duplicate block from a miner, so yes, this would be DoS. @gmaxwell can you see what I’m saying here?
    gmaxwell commented at 8:51 am on September 4, 2014:

    I’m responding to the comments, not the code— if it’s only on duplicates and not just non-requested then I think its safe but only if we release note loudly. Right now I think eloipool users will end up GBT submitting blocks to multiple Bitcoinds which will then aggressively relay. It’s bandwidth wasteful and it shouldn’t be permitted. This is a borderline p2p protocol change, I think.

    Though relaynodeclient should do an INV— it’s local, this is going to add like… 1ms. And it would also let the client better track when it’s the software providing the block. Same with p2pool. Though indeed, whitelisting also covers these cases.

    rebroad commented at 9:00 am on September 4, 2014:
    @sipa However, you do still need to make sure not to DoS a node for sending a block that you’ve requested. So far, this logic isn’t in there. This happened to me today when I got the inv from the non-miner, but the miner sent the block directly and it arrived from them first. My node requested the block from the node that sent the inv, and then DoSed it when it received it - it was only doing what it was asked to, so shouldn’t have been DoSed.
    sipa commented at 9:16 am on September 4, 2014:

    @rebroad That’s a good point. I’m not sure how to deal with that correctly.

    • Making an exception for allowing a duplicate block to be received if we actually requested it is possible, but requires extra logic.
    • Not DoS/disconnecting in case of any duplicate is easy, but doesn’t really encourage low bandwidth operation.

    Previously my position was that sending-without-inv was safe in case you know the peer does not have the data yet. Your case is a good counterexample for that, and perhaps the right solution is indeed just to argue for a protocol change that makes sending-without-inv illegal.

    gmaxwell commented at 9:26 am on September 4, 2014:
    Perhaps define that as of protocol X we’ll never do that ourselves anymore, and enforce it. Later minimum support advances past protocol X. Seems like such a minor change to bother staging…
    rebroad commented at 10:45 am on September 4, 2014:

    @sipa more simply, miners can still send the block without it being requested, as long as they send the “inv” just before it (so that nodes don’t think they’re not receiving the block until it arrives). This way they’ll very unlikely get banned since their invs will almost always be received before anyone else’s.

    The code can be changed easily to do what it did before, which is to record which node sent the block and compare if it was the node that it was requested from. Then just change the DoS logic over to the node that sent it unsolicted rather than the node that the block arrived from later.

    The node that sent me the block without an inv was public.au.relay.mattcorallo.com:8335 - so maybe someone can submit a pull request to his software so that invs are sent immediatley before the block is sent. @TheBlueMatt

  11. laanwj commented at 10:55 am on July 7, 2014: member

    Synced in 3:43 here, and with lots of other things running on the same computer.

    I do get this error when running with -checkblocks=0 -checklevel=4 afterwards (after shutting down with ‘stop’):

    02014-07-07 10:48:02 Verifying last 309612 blocks at level 4
12014-07-07 10:48:03 ERROR: ReadFromDisk : Deserialize or I/O error - CAutoFile::read : end of file
22014-07-07 10:48:03 ERROR: VerifyDB() : *** found bad undo data at 309606, hash=000000000000000013e7146f5a1f63a188935bedc491b8b5bf5ab823f6ec0d9c

    Oh I get the same without any checklevel options. I’ll keep this copy of the blocks data and database in case it’s interesting for debuging.

  12. in src/main.cpp: in eb7822c91b outdated 
    3776+                    // doing this will result in the received block being rejected as an orphan.
3777+                    pfrom->PushMessage("getheaders", chainActive.GetLocator(pindexBestHeader), inv.hash);
3778+                    if (Params().NetworkID() == CBaseChainParams::REGTEST ||
3779+                        chainActive.Tip()->GetBlockTime() > GetAdjustedTime() - Params().TargetSpacing() * 20) {
3780+                        vToFetch.push_back(inv);
3781+                        MarkBlockAsInFlight(pfrom->GetId(), inv.hash);
    sipa commented at 12:22 pm on July 7, 2014:
    We already download every block that peers advertize. In PR changes it to first ask for headers, and only asks for the block immediately if we’re close to being synced. Validating the header first would require an extra roundtrip, which can hurt network propagation time.
  13. in src/main.cpp: in eb7822c91b outdated 
    3772+                    // there are no such headers.
3773+                    // Secondly, and only when we are close to being synced, we request the announced block afterwards,
3774+                    // to avoid an extra round-trip. Note that we must *first* ask for the headers, so by the
3775+                    // time the block arrives, the header chain leading up to it is already validated. Not
3776+                    // doing this will result in the received block being rejected as an orphan.
3777+                    pfrom->PushMessage("getheaders", chainActive.GetLocator(pindexBestHeader), inv.hash);
    sipa commented at 12:24 pm on July 7, 2014:
    We do, for initial sync (see SendMessages). This code is for dealing with newly announced blocks so they can propagate.
    gmaxwell commented at 12:27 pm on July 7, 2014:
    It does, under “// Start block sync” in SendMessages(). The code you’re commenting on is for newly announced blocks. Not that it’s relevant to an inv.hash, so it’s not something we can do before even hearing something from the peer.
  14. in src/main.cpp: in eb7822c91b outdated 
    2671@@ -2624,93 +2672,25 @@ void CBlockIndex::BuildSkip()
2672         pskip = pprev->GetAncestor(GetSkipHeight(nHeight));
2673 }
2674 
2675-void PushGetBlocks(CNode* pnode, CBlockIndex* pindexBegin, uint256 hashEnd)
    sipa commented at 12:26 pm on July 7, 2014:
    We use getheaders instead of getblocks. See SendMessages.
  15. sipa commented at 3:04 pm on July 8, 2014: member
    Changed the code to use block-first-seen rather than header-first-seen to distinguish between equal-work branches.
  16. sipa commented at 3:37 pm on July 11, 2014: member
    Rebased on top of #4496 and #4497 (which were already included here).
  17. sipa commented at 10:23 pm on July 11, 2014: member
    Moved RPC changes to a separate commit (now the actual headers-first commit is a net negative in lines, while adding comments!).
  18. laanwj added this to the milestone 0.10.0 on Jul 14, 2014
  19. gmaxwell commented at 1:10 pm on July 14, 2014: contributor
    Needs rebase.
  20. sipa commented at 2:21 pm on July 14, 2014: member
    Rebased.
  21. laanwj commented at 6:52 am on July 15, 2014: member
    I retried my testing, now with a non-corrupting destination, and found no issues. I tickled it in various ways with -checklevel -checkblocks and it completed fine.
  22. rebroad commented at 10:34 am on July 16, 2014: contributor
    Ok, I have a thought/question. With this pull, won’t it effectively mean that the average height of the average node be less than without this pull? I.e. detrimental to the bitcoin network?
  23. gmaxwell commented at 11:55 am on July 16, 2014: contributor
    @rebroad I can’t figure out why you’d think that. Once synchronized the heights of all nodes will be the best available to them, and this change makes the system synchronize much faster.
  24. rebroad commented at 10:51 am on July 17, 2014: contributor
    It’s not the headers first aspect that makes them sync faster but rather the use of concurrent block downloads. The getting of headers first and the downloading of blocks that aren’t necessarily in the best chain will delay nodes getting up to date.
  25. laanwj commented at 11:04 am on July 17, 2014: member
    I’m not convinced by your claim @rebroad. Let’s look at the evidence here: with this code, new nodes get up to speed much faster. With a decent internet connection this is scarcely longer than a -reindex would take. I don’t see one single case of a user reporting that this makes a node get up to date slower.
  26. gmaxwell commented at 11:48 am on July 17, 2014: contributor
    @rebroad Part of the whole point is that it can use the headers to determine the best chain (with very high probability— only invalidated if the majority hashrate chain is invalidated) very fast, then it only downloads blocks in the best chain. New blocks at the tip are downloaded like they’ve always been.
  27. laanwj commented at 11:51 am on July 17, 2014: member
    Right, this mostly avoids downloading blocks not on the best chain unlike before. No more orphans…
  28. sipa commented at 2:29 pm on July 17, 2014: member
    In the steady state, headers and blocks are fetched in a single request (unless there is a reorg), so there are no additional roundtrips for getting synchronized. There’s an extra 100 bytes or so to get the header…
  29. in src/main.cpp: in 5b6b078b27 outdated 
    89@@ -96,8 +90,14 @@ namespace {
90     };
91 
92     CBlockIndex *pindexBestInvalid;
93-    // may contain all CBlockIndex*'s that have validness >=BLOCK_VALID_TRANSACTIONS, and must contain those who aren't failed
94+
95+    // The set of all CBlockIndex entries with BLOCK_VALID_TRANSACTIONS or better that are at least
96+    // as good as our current tip. Entries may be failed, though.
    laanwj commented at 7:27 am on July 23, 2014:
    What does ‘may be failed’ mean in this context? The block has valid transactions, but failed the higher checks?
    sipa commented at 8:48 am on July 23, 2014:

    Indeed. Feel free to suggest better wording.

    The reason is that we don’t have a map from blocks to their successors, only backwards. That means that if there’s a fork A-B-C and A-B-D-E, and we try to connect E (which fails at B), A-B-D-E are marked failed (and E is removed from setBlockIndexValid), but C isn’t marked failed or removed from setBlockIndexValid. This is detected as soon as we do try to connect C.

    laanwj commented at 9:51 am on July 23, 2014:

    Thanks for explaining, that makes it clear to me.

    I think the wording is fine, but it would be useful to add that explanation in the comment as well.

  30. in src/main.cpp: in 5b6b078b27 outdated 
    355+    if (pa == pb)
356+        return pa;
357+    return NULL;
358+}
359+
360+/** Update pindexLastCommonBlock and return its not-in-flight missing successors. */
    laanwj commented at 1:24 pm on July 23, 2014:
    Precondition: vBlocks.empty() ?
    sipa commented at 2:16 pm on July 23, 2014:
    Is it fine if I change the description to ‘append its not-in-flight missing successors to vBlocks’?
    laanwj commented at 2:19 pm on July 23, 2014:
    Well, you’re making the amount of blocks added depend on the current vBlocks.size(), so it appeared to me that you assume you’re starting with an empty vector.
    sipa commented at 2:24 pm on July 23, 2014:
    Update pindexLastCommonBlock and its not-in-flight missing successors to vBlocks, until that vector has at most count entries.
    laanwj commented at 2:28 pm on July 23, 2014:
    OK
  31. in src/main.cpp: in 5b6b078b27 outdated 
    2254-         LOCK(cs_nBlockSequenceId);
2255-         pindexNew->nSequenceId = nBlockSequenceId++;
2256-    }
2257+    // We assign the sequence id to blocks only when the full data is available,
2258+    // to avoid miners withholding blocks but broadcasting headers, to get a
2259+    // competitive advantage.
    laanwj commented at 1:43 pm on July 23, 2014:
    Good idea.
  32. in src/main.cpp: in 5b6b078b27 outdated 
    2436+    // These are checks that are independent of context.
2437 
2438     if (!CheckBlockHeader(block, state, fCheckPOW))
2439         return false;
2440 
2441+    // Check merkle root
    laanwj commented at 1:56 pm on July 23, 2014:
    Any specific reason for moving this before checking ‘size limits’?
    sipa commented at 2:15 pm on July 23, 2014:
    Yes, see the comment added before the ‘size limits’ section.
    laanwj commented at 3:15 pm on July 23, 2014:
    Ah, makes sense.
  33. in src/main.cpp: in 5b6b078b27 outdated 
    2974@@ -2973,13 +2975,26 @@ bool static LoadBlockIndexDB()
2975     {
2976         CBlockIndex* pindex = item.second;
2977         pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + pindex->GetBlockWork();
2978-        pindex->nChainTx = (pindex->pprev ? pindex->pprev->nChainTx : 0) + pindex->nTx;
2979-        if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS))
2980+        if (pindex->nStatus & BLOCK_HAVE_DATA) {
2981+            if (pindex->pprev) {
2982+                if (pindex->pprev->nChainTx) {
    laanwj commented at 2:04 pm on July 23, 2014:
    What is the reason for comparing pindex->pprev->nChainTx to 0 here? Is this a special value that means ‘unlinked block’?
    sipa commented at 2:14 pm on July 23, 2014:
    Indeed. There’s a comment on BLOCK_VALID_TRANSACTIONS explaining that. I’ll add it to nChainTx itself.
    laanwj commented at 3:17 pm on July 23, 2014:
    Right. For a moment I thought this was the case for blocks that are the successor of the genesis block, but nChainTx is inclusive so even the genesis block has nChainTx=1 because of the coinbase transaction.
  34. in src/version.h: in 5b6b078b27 outdated 
    30@@ -31,8 +31,11 @@ static const int PROTOCOL_VERSION = 70002;
31 // intial proto version, to be increased after version/verack negotiation
32 static const int INIT_PROTO_VERSION = 209;
33 
34+// In this version, 'getheaders' was introduced.
35+static const int GETHEADERS_VERSION = 31800;
36+
37 // disconnect from peers older than this proto version
38-static const int MIN_PEER_PROTO_VERSION = 209;
39+static const int MIN_PEER_PROTO_VERSION = GETHEADERS_VERSION;
    laanwj commented at 4:24 pm on July 23, 2014:
    Protocol version 31800 was version 0.3.18. It’s absolutely reasonable to bump the minimum peer version to that.
    gmaxwell commented at 6:04 am on July 24, 2014:
    NACK rebroad’s NACK. 0.3.18 over 3.5 years old, and ACK the change. Any systems actually running that software are no longer in-sync with the blockchain unless configuration modified and have numerous security vulnerabilities. Non-bitcoin core claiming that version number or before is still enforcing an old version of the protocol without modern facilities like ping. There is no substantial deployment of versions this old any longer: as of today Luke’s spider reports zero nodes running code that old. Finally, continuing to support pre-getheader systems is just more cases that would need to be tested.
    laanwj commented at 6:13 am on July 24, 2014:
    It’s better if such old versions just fail to connect to the network, than that they pretend to catch up and fail due to some other reason.
  35. sipa commented at 1:09 am on July 24, 2014: member
    Rebased and addressed some comments by @mikehearn and @laanwj.
  36. sipa commented at 8:10 pm on July 26, 2014: member
    Modified the fetch-block selection a bit: the download window does not start one past the current active tip anymore, but at the first block we don’t have in common with a particular peer. The previous algorithm would fail to reorg further back than the download window. Also added extra comments and asserts.
  37. laanwj added the label P2P on Jul 31, 2014
  38. laanwj added the label Improvement on Jul 31, 2014
  39. laanwj added the label Priority High on Jul 31, 2014
  40. sipa force-pushed on Aug 23, 2014
  41. sipa commented at 11:55 pm on August 23, 2014: member
    Rebased.
  42. sipa force-pushed on Aug 26, 2014
  43. sipa force-pushed on Aug 27, 2014
  44. in src/main.cpp: in 2525afb58a outdated 
    383+        // Read up to 128 (or more, if more blocks than that are needed) successors of pindexWalk (towards
384+        // pindexBestKnownBlock) into vToFetch, but never any beyond pindexBestKnownBlock or BLOCK_DOWNLOAD_WINDOW
385+        // beyond our current tip. We fetch 64, because CBlockIndex::GetAncestor may be as expensive as iterating over
386+        // ~100 CBlockIndex* entries anyway.
387+        int nMaxHeight = std::min<int>(state->pindexBestKnownBlock->nHeight, chainActive.Height() + BLOCK_DOWNLOAD_WINDOW);
388+        int nToFetch = std::min(nMaxHeight - pindexWalk->nHeight, std::max<int>(vBlocks.size() - count, 128));
    sipa commented at 4:25 pm on September 2, 2014:
    Read a few lines before…
  45. in src/main.h: in 2525afb58a outdated 
    619+
620+    // Scripts & signatures ok. Implies all parents are also at least SCRIPTS.
621+    BLOCK_VALID_SCRIPTS      =    5,
622+
623+    // All validity bits.
624     BLOCK_VALID_MASK         =    7,
    rebroad commented at 4:31 pm on September 2, 2014:
    What about 6?
    sipa commented at 4:33 pm on September 2, 2014:

    There is no 6, nor is there a 7.

    MASK is just the boolean OR of the bits used for validity.

    jgarzik commented at 4:45 pm on September 2, 2014:

    Minor nit, since it got highlighted: usual preference is for such a mask to be composed of prior constants, rather than a hardcoded number itself. The exception to this rule is usually if you just want the field to store all 1’s (0xffffffff).

    Sometimes it’s nice to have the constants define the actual bits (“1U « $n”), but it depends on the downstream code whether or not that makes downstream code more or less clean.

    sipa commented at 4:58 pm on September 2, 2014:
    @jgarzik See #4820.
  46. in src/main.cpp: in 2525afb58a outdated 
    3986+        }
3987+
3988+        if (pindexLast)
3989+            UpdateBlockAvailability(pfrom->GetId(), pindexLast->GetBlockHash());
3990+
3991+        if (nCount == MAX_HEADERS_RESULTS && pindexLast) {
    rebroad commented at 1:09 am on September 3, 2014:

    This is an assumption. This will only work if the other node is the default bitcoind with the same MAX_HEADERS_RESULTS. It’s entirely possible that other nodes will have a different maximum setting, in which case, the logic here fails.

    Maybe instead it’s better to keep requesting until the results come back smaller than the previous results (this way no assumptions need to be made other than the assumption that their MAX is fixed rather than dynamic).

    I’d also suggest only requesting more headers if the headers received so far were for blocks that we didn’t already have.

    sipa commented at 8:40 am on September 4, 2014:
    Making MAX_HEADERS_RESULTS dynamic is possible, but it would be a protocol change. Right now, it is correct w.r.t. other nodes out there.
  47. in src/main.cpp: in 2525afb58a outdated 
    4488+        if (!state.fSyncStarted && !pto->fClient && !pto->fInbound && !fImporting && !fReindex) {
4489+            // Only actively request headers from a single peer, unless we're close to today.
4490+            if (nSyncStarted == 0 || pindexBestHeader->GetBlockTime() > GetAdjustedTime() - 24 * 60 * 60) {
4491+                state.fSyncStarted = true;
4492+                nSyncStarted++;
4493+                pto->PushMessage("getheaders", chainActive.GetLocator(pindexBestHeader->pprev), uint256(0));
    rebroad commented at 1:17 am on September 3, 2014:

    Do you want to be requesting headers from peers whose advertised block height is lower than our node’s? They won’t regognize the locator so they’ll end up sending everything they have, which would surely be a waste of bandwidth and rarely helpful to our node.

    Also, might it be better to change the default response to getheaders requests so that this doesn’t happen?

    gmaxwell commented at 1:20 am on September 3, 2014:
    What happens when I send you a 400,000 header difficulty 1 fork (as an example) before you query someone else? A lower height does not necessarily mean a less preferable chain.
    rebroad commented at 1:38 am on September 3, 2014:
    @gmaxwell a 400000 header difficulty 1 fork would have a higher block height than our node. I’m suggesting we don’t request headers from nodes with a lower block height than our node. The logic I’m proposing is in line with the previous logic the code had - i.e. we didn’t getblocks from peers that had a lower height.
    gmaxwell commented at 1:40 am on September 3, 2014:
    I understood what you were suggesting. If we’re on the fork with more blocks we would never reorganize off it with your suggestion.
    sipa commented at 8:43 am on September 4, 2014:
    @rebroad Locators contain block hashes of exponentially increasing age; it’s very unlikely they’ll send everything they have. There are certainly possible heuristics to improve this, but making sure it works correctly has priority now.
    rebroad commented at 1:54 am on September 6, 2014:

    Also, why send the pprev rather than the latest as the locator?

    I still don’t understand why we would want to get only 1 header from up-to-date nodes, but tens of thousands of headers from not-up-to-date nodes…

    rebroad commented at 11:38 am on September 7, 2014:
    @sipa I’m unfamiliar the locators to be honest, but surely it would make more sense to send the getheaders request using a locator that won’t be unrecognised, e.g. send the locator corresponding to the block height the we know the node has. It’s then far more likely to send us just it’s last block header rather than going back to thousands of blocks that we already know of.
    sipa commented at 11:48 am on September 7, 2014:
    That would mean downloading all headers from all peers. Initially, we don’t know anything about our peers, and it is exactly through getheaders (and a bit through invs, but that’s harder to rely on) that we learn about what the peer knows. Usually, when sending an initial getheaders request to a peer, they already have the majority of headers we want, and using our own best will in the average case result in only a few 100 headers.
    sipa commented at 11:51 am on September 7, 2014:
    Sending pprev: so that we get some header back as a result in case they are at the same point as us, as we use received headers to update our information about what the peer knows.
    rebroad commented at 3:41 am on September 10, 2014:
    @sipa I’m not sure you’re understanding what I’m trying to say. Clearly if a node is at height 295403, it’s not going to recognise the locator for block 321032, so why bother sending it a locator we know it won’t recognise? Why not sent it the locator for block 295403 instead?
    rebroad commented at 3:51 am on September 11, 2014:
    I’d like to submit a patch for this, but I’m not sure how to find the locator for a block just from it’s height - could someone help me find an example where this is done please?
    sipa commented at 6:13 am on September 11, 2014:
    Please understand what locators are before commenting further on this. They are exactly designed to keep working if either party is ahead from the other. And yes, it will be a bit less efficient if there is a difference, but we don’t know how far the other peer is. We only know they are at least at 295…; we send the getheaders exactly to find out how much further they are.
  48. in src/main.h: in 2525afb58a outdated 
    73+static const int MAX_BLOCKS_IN_TRANSIT_PER_PEER = 16;
74+/** Timeout in seconds during which a peer must stall block download progress before being disconnected. */
75+static const unsigned int BLOCK_STALLING_TIMEOUT = 2;
76+/** Number of headers sent in one getheaders result. We rely on the assumption that if a peer sends
77+ *  less than this number, we reached their tip. Changing this value is a protocol upgrade. */
78+static const unsigned int MAX_HEADERS_RESULTS = 2000;
    gmaxwell commented at 1:36 am on September 3, 2014:
    Because thats how GETHEADERS is defined in the protocol, as the comment notes— changing it is an incompatible version change. See: https://en.bitcoin.it/wiki/Network#Messages. There needs to be some limit, or excessive memory consumption or message blocking happens.
    jgarzik commented at 2:26 am on September 3, 2014:
    The protocol is defined in the source code. Satoshi added getheaders a long time ago.
    gmaxwell commented at 2:28 am on September 3, 2014:
    The whitepaper just explains the high level concept of the Bitcoin consensus protocol. It describes none of the implementation details, and especially doesn’t describe the P2P protocol.
    sipa commented at 11:59 am on September 7, 2014:
    getheaders, in a way compatible with how this PR uses it, was added in december 2010 (Bitcoin 0.3.18) by satoshi.
  49. in src/main.cpp: in 2525afb58a outdated 
    2533@@ -2480,6 +2534,10 @@ bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex,
2534     if (!AcceptBlockHeader(block, state, &pindex))
2535         return false;
2536 
2537+    if (pindex->nStatus & BLOCK_HAVE_DATA) {
2538+        return state.DoS(20, error("AcceptBlock() : already have block %d %s", pindex->nHeight, pindex->GetBlockHash().ToString()), REJECT_DUPLICATE, "duplicate");
    rebroad commented at 4:24 am on September 4, 2014:
    Ok, I’m noticing this isn’t ideal since using matt’s bitcoinj node which sends the block before the node my node requests the block from sends it. Therefore, I’d suggest refining this to only consider it dos if it’s not an “addnode” node AND we didn’t request the block from this node.
    sipa commented at 8:44 am on September 4, 2014:
    I would suggest using the whitelisting mechanism for that.
  50. rebroad commented at 5:07 am on September 4, 2014: contributor
    @sipa Regarding this pull breaking re-orgs - why doesn’t it, instead of adding out of order blocks to the leveldb, deal with those blocks in the same way it used to do with orphans? (or a temporary database where they are deleted when added to the block index).
  51. gmaxwell commented at 5:12 am on September 4, 2014: contributor
    This pull does not break re-orgs. (And as an aside, storing blocks in memory is not acceptable because it may cause very high peak memory usage)
  52. rebroad commented at 7:46 am on September 4, 2014: contributor
    @gmaxwell Sorry, I meant reindexing. Yes, I agree not good to store in memory. I don’t know enough about why it breaks reindexing, but I’m assuming a separate database would be the answer (for the out-of-order blocks, which would then be added to the main database in order),.
  53. gmaxwell commented at 7:52 am on September 4, 2014: contributor
    Reindexing should just handle out of order blocks exactly as headers first does, no need for a separate database. Reindexing works like a peer feeding us blocks, with headers first it should be made like the headers first process— e.g. scan the blocks on disk to learn their headers, add them to the block index, and then connect them as we can.
  54. sipa commented at 9:02 am on September 4, 2014: member
    @gmaxwell That’s not exactly true. When receiving from network, we always have all the parent headers before processing a block, so it can just be entered into our block tree. When reindexing, that is not the case, and there is actually need for some handling of “orphan headers”.
  55. sipa force-pushed on Sep 4, 2014
  56. sipa commented at 9:49 am on September 4, 2014: member
    Rebased and removed the DoS banning for duplicate blocks (but left as a TODO).
  57. rebroad commented at 10:53 am on September 4, 2014: contributor
    @sipa Shall I provide a patch to the duplicate block DoS stuff and submit it as a pull request to this branch?
  58. sipa force-pushed on Sep 4, 2014
  59. rebroad commented at 6:09 am on September 5, 2014: contributor

    The stalled block detection math is wrong I think:-

    2014-09-05 05:52:50 inv (get) block 00000000000000000ecadcd9cfa2f30ae66783aa07b53355f97f1b90a4999694 from peer=95 2014-09-05 05:52:50 received 1 headers from peer=95 2014-09-05 05:52:50 Stall started peer=95 2014-09-05 05:52:53 nNow=1409896373020224 nStallingSince=1409896370957264 2014-09-05 05:52:53 Peer=95 is stalling block download, disconnecting 2014-09-05 05:52:53 disconnecting peer=95

    I added a couple of debug lines. Looks like the 1000000 difference is too small as this equates to about 3 seconds rather than 2 minutes.

    By the way, why use micros rather than millis?

  60. rebroad commented at 6:49 am on September 6, 2014: contributor

    Found a bug - it getdatas blocks even when the same block has already been received (but not processed). debug.log extract:-

    2014-09-06 05:56:20 inv (get) block 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb from peer=18 2014-09-06 05:56:20 getheaders (319311) 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb to peer=18 2014-09-06 05:56:21 inv (get) block 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb from peer=13 2014-09-06 05:56:21 getheaders (319311) 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb to peer=13 2014-09-06 05:56:21 received 7 headers (319312 to 319318) from peer=18 (startheight:319317) 2014-09-06 05:56:21 Requesting block 0000000000000000164de10a34768067aeaa95a0790472015d928b9bee4caeb8 (319310) peer=18 2014-09-06 05:56:21 Requesting block 00000000000000002720bb210d88b74be81df1eaee857a287cb6adcb027c5ef8 (319311) peer=18 <–snip–> 2014-09-06 05:56:21 Requesting block 000000000000000011165e5ecfd4c7f26cca6c29cce97e0533d047a562ef2888 (319317) peer=18 2014-09-06 05:56:21 inv block 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb (height:319318) from peer=4 2014-09-06 05:56:21 received 7 headers (319312 to 319318) from peer=13 (startheight:319317) 2014-09-06 05:56:22 received block 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb peer=18 2014-09-06 05:56:27 received block 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb peer=13 2014-09-06 05:56:27 ERROR: AcceptBlock() : already have block 319318 000000000000000002498192998dc2fbe440f3d37282e8e5b3c2ab19873a3ecb 2014-09-06 05:56:27 ERROR: ProcessBlock() : AcceptBlock FAILED 2014-09-06 05:56:28 received block 0000000000000000164de10a34768067aeaa95a0790472015d928b9bee4caeb8 peer=18

    Curiously, the getdata block was done without the corresponding “Requesting block” logged…

  61. rebroad commented at 6:57 am on September 6, 2014: contributor
    Another interesting feature is that when a new block inv is seen, it both getheaders the node and getdata’s the block in the hope that the headers arrive first. However, if the block arrives before the headers, and the previous block hasn’t been fetched yet, it will actually DoS the node the sent the block because it doesn’t have the previous block…
  62. sipa force-pushed on Sep 6, 2014
  63. in src/main.cpp: in 185fa1d823 outdated 
    2351         pindex = miSelf->second;
2352+        if (ppindex)
2353+            *ppindex = pindex;
2354         if (pindex->nStatus & BLOCK_FAILED_MASK)
2355             return state.Invalid(error("AcceptBlock() : block is marked invalid"), 0, "duplicate");
2356+        return true;
    sipa commented at 3:48 pm on September 6, 2014:
    Headers do not contain the number of transactions, and just changing the protocol to also report it along with headers is pretty pointless, as the data is not verifiable.
    rebroad commented at 8:40 am on September 7, 2014:
    https://en.bitcoin.it/wiki/Header#Block_Headers says that there is a field for the number of transactions - why isn’t this used?
    sipa commented at 10:58 am on September 7, 2014:
    It lists the number of transactions contained within the same message. For ‘headers’, this number is always 0. This is because historically, CBlockHeader did not exist, and just a CBlock without transactions was sent.
  64. sipa commented at 3:53 pm on September 6, 2014: member
    @rebroad You’re right to point out that the request of headers + getdata of an inved block simultaneously makes an assumption about the order in which the peer will process them. However, it’s a very reasonable assumption to make, as there are already systems that rely on this assumption (though not for block propagation). I would argue that it’s already implicitly part of the protocol definition.
  65. sipa commented at 3:58 pm on September 6, 2014: member

    @rebroad I fixed the stalling detection (still testing it, now). The older version sometimes triggered when there was just a single block in flight, which is silly. Stalling peer P is now strictly defined as “we can’t download anything from peer Q (!= P), because all blocks in the download window are in flight, and we’re waiting for a block from P for the window to move”.

    The timeout value is correct, however, it’s intended to be just 2 seconds (see the comment above the definition, even). This is just to prevent us from disconnecting peers because we haven’t had a chance to process anything from them.

  66. sipa commented at 4:13 pm on September 6, 2014: member

    @rebroad If a block is ever simultaneously requested multiple times, that’s a bug. Looking at the code, I don’t see how that would be possible. Both block getdata code paths (direct request on inv when caught up, and from sendmessages as part of the normal parallel block fetching) check both whether it’s not already marked BLOCK_HAVE_DATA or in mapBlocksInFlight, and the transition between them (in ProcessBlock) happens while holding cs_main.

    Please report if you see that happen again.

  67. sipa force-pushed on Sep 6, 2014
  68. sipa commented at 7:18 pm on September 6, 2014: member
    Added a commit: removing pow’s CheckMinWork, which verified whether an nBits/nTime was possible, without knowing the parents. As we always know all headers before validation, this is no longer necessary.
  69. sipa force-pushed on Sep 6, 2014
  70. in src/main.cpp: in 33bdf0da33 outdated 
    402+            if (pindex->nStatus & BLOCK_HAVE_DATA) {
403+                if (pindex->nChainTx)
404+                    state->pindexLastCommonBlock = pindex;
405+            } else if (mapBlocksInFlight.count(pindex->GetBlockHash()) == 0) {
406+                // The block is not already downloaded, and not yet in flight.
407+                if ((int)pindex->nHeight > chainActive.Height() + BLOCK_DOWNLOAD_WINDOW) {
    Arnavion commented at 3:59 am on September 7, 2014: 
    0main.cpp: In function 'void {anonymous}::FindNextBlocksToDownload(NodeId, unsigned int, std::vector<CBlockIndex*>&, NodeId&)':
1main.cpp:407:67: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
2                 if ((int)pindex->nHeight > chainActive.Height() + BLOCK_DOWNLOAD_WINDOW) {
3                                                                   ^
    rebroad commented at 3:53 am on September 11, 2014:
    changing the (int) to (unsigned int) fixes this compile warning.
  71. in src/main.cpp: in 33bdf0da33 outdated 
    2359-    }
2360-
2361-    CBlockIndex* pcheckpoint = Checkpoints::GetLastCheckpoint();
2362-    if (pcheckpoint && block.hashPrevBlock != (chainActive.Tip() ? chainActive.Tip()->GetBlockHash() : uint256(0)))
2363-    {
2364-        // Extra checks to prevent "fill up memory by spamming with bogus blocks"
    rebroad commented at 11:18 am on September 7, 2014:
    we don’t need to prevent memory fill due to spam any longer?
    sipa commented at 11:35 am on September 7, 2014:
    We certainly do, but every case this protects against is now caught in the proof of work check below. Previously, this check was useful, because we wanted to verify whether a block was viable without knowing its parent. In headers-first, we always know all headers before processing a block, so it doesn’t add anything anymore.
    rebroad commented at 3:22 pm on September 7, 2014:
    What about when a block is sent without a header or inv first?
    sipa commented at 3:24 pm on September 7, 2014:
    If it connects cleanly to the chain we already have, there is no problem. If not, it is discarded. That’s the basic idea behind headers-first: we can’t accept a block without knowing (the headers of) its ancestors.
    sipa commented at 6:11 am on September 11, 2014:
    We call AcceptBlockHeader even for blocks received before having received their header. And the proof of work check further on is stronger than this one.
  72. in src/main.cpp: in 33bdf0da33 outdated 
    2371-        if (!CheckMinWork(block.nBits, pcheckpoint->nBits, deltaTime))
2372-        {
2373-            return state.DoS(100, error("CheckBlockHeader() : block with too little proof-of-work"),
2374-                             REJECT_INVALID, "bad-diffbits");
2375-        }
2376+        return true;
    rebroad commented at 11:20 am on September 7, 2014:

    Ah, I think I was thinking headers were more like the things mentioned in https://bitcointalk.org/index.php?topic=145066.0

    What happened to the headers described in that article? It sounded like an excellent idea.

    sipa commented at 11:34 am on September 7, 2014:
    That article describes a novel network protocol. It’s not what is implemented.
    rebroad commented at 3:44 am on September 10, 2014:
    Any plans to implement that? What would be the next step? Someone to submit a pull request implementing it?
  73. in src/main.cpp: in 33bdf0da33 outdated 
    3888 
3889-        LogPrint("net", "received block %s peer=%d\n", block.GetHash().ToString(), pfrom->id);
3890-
3891         CInv inv(MSG_BLOCK, block.GetHash());
3892-        pfrom->AddInventoryKnown(inv);
3893+        LogPrint("net", "received block %s peer=%d\n", inv.hash.ToString(), pfrom->id);
    rebroad commented at 11:29 am on September 7, 2014:

    LogPrint("net", "received %s peer=%d\n", inv.ToString(), pfrom->id); would be shorter :)

    Also, I notice you sometimes use pfrom->id and sometimes pfrom->GetId() - what’s the advantage in the latter one?

    sipa commented at 11:43 am on September 7, 2014:

    Just good practice in general; you shouldn’t be looking at fields of complex data structures. Using a wrapper method means more flexibility about changing the implementation later.

    Then again, it’s pretty meaningless if not done consistently.

  74. in src/main.cpp: in 33bdf0da33 outdated 
    2415@@ -2376,6 +2416,12 @@ bool AcceptBlock(CBlock& block, CValidationState& state, CBlockIndex** ppindex,
2416     if (!AcceptBlockHeader(block, state, &pindex))
2417         return false;
2418 
2419+    if (pindex->nStatus & BLOCK_HAVE_DATA) {
2420+        // TODO: deal better with duplicate blocks.
2421+        // return state.DoS(20, error("AcceptBlock() : already have block %d %s", pindex->nHeight, pindex->GetBlockHash().ToString()), REJECT_DUPLICATE, "duplicate");
2422+        return true;
    sipa commented at 11:33 am on September 7, 2014:
    If false is returned, some error condition should be set in state.
  75. in src/main.cpp: in 33bdf0da33 outdated 
    2329@@ -2262,16 +2330,6 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo
2330         if (!CheckTransaction(tx, state))
2331             return error("CheckBlock() : CheckTransaction failed");
2332 
2333-    // Check for duplicate txids. This is caught by ConnectInputs(),
2334-    // but catching it earlier avoids a potential DoS attack:
    sipa commented at 11:36 am on September 7, 2014:
    It was moved up a few lines. There’s even a comment about why it was moved.
  76. sipa commented at 12:34 pm on September 7, 2014: member

    @rebroad regarding reindexing: it’s not that we’re adding blocks to leveldb out of order (they’re only applied to the chainstate when being validated, which happens in-order), but they’re written to the blk*.dat files in the order they are downloaded (after validating the headers). The current reindexing logic (and that of earlier versions) assumes that the files on disk contain blocks in order, so that breaks it indeed.

    The reason for presenting this pull request without having the reindexing logic in it: I don’t expect it to be hard or controversial to add, but believe that getting comments on the core validation/downloading logic is way more important.

    Perhaps that makes it non-mergable right now, but it’s not mergable anyway as long as there is no test infrastructure in place that can deal with it.

  77. sipa force-pushed on Sep 9, 2014
  78. sipa force-pushed on Sep 9, 2014
  79. BitcoinPullTester commented at 11:56 pm on September 9, 2014: none

    Automatic sanity-testing: FAILED MERGE, see http://jenkins.bluematt.me/pull-tester/p4468_c248436f60f18bc2ee886c912872a124212d04b9/ for test log.

    This pull does not merge cleanly onto current master 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.

  80. in src/main.cpp: in 2692f57279 outdated 
    3627+                pfrom->AskFor(inv);
3628 
3629-            if (inv.type == MSG_BLOCK)
3630+            if (inv.type == MSG_BLOCK) {
3631                 UpdateBlockAvailability(pfrom->GetId(), inv.hash);
3632+                if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) {
    rebroad commented at 3:46 am on September 11, 2014:
    What if the peer that the “getheaders” has been sent to ignores the request? The logic doesn’t seem to cater for that and assumes that one “getheaders” request to the first node sending the inv will be sufficient. By the peer ignoring the getheaders request, this could cause the requesting node to have to wait until the next mined block before it can request the missing block, and if the same node again sends the first inv, this could cause the node to get stuck behind for quite some time.
    sipa commented at 6:19 am on September 11, 2014:
    Yes that is a concern. It’s not more a concern than a peer currently ignoring getblocks however when it is our syncnode. Eventually we’ll learn from other peers what the best headers are, when they announce a new block. This is just to speed things up. It’s always a compromise between bandwidth and reliability. Sending the request to two peers will make it safer, but also means downloading all headers twice in the worst case…
  81. sipa force-pushed on Sep 12, 2014
  82. in src/main.cpp: in 9722e9ed1f outdated 
    4537+                    pindex->nHeight, pto->id);
4538+            }
4539+            if (staller != -1) {
4540+                if (State(staller)->nStallingSince == 0) {
4541+                    State(staller)->nStallingSince = nNow;
4542+                    LogPrint("net", "Stall started peer=%d\n", staller);
    rebroad commented at 1:57 pm on September 17, 2014:

    Something seems to be wrong with the stalling logic, judging from my debug.log output:-

    2014-09-17 13:38:05 received block 0000000000000838ff0a36baecd1d4c07807ebc54a59e7e05dd4681839ba2827 (height:191655) peer=103 2014-09-17 13:38:06 Requesting block 00000000000004f2b85c51e890bd7a445d7cbb16469c38447ab557c07eaafdc2 (192679) peer=103 2014-09-17 13:38:07 Stall started peer=103 2014-09-17 13:38:09 Peer=103 is stalling block download, disconnecting 2014-09-17 13:38:09 disconnecting peer=103

  83. sipa force-pushed on Sep 17, 2014
  84. sipa force-pushed on Sep 19, 2014
  85. sipa force-pushed on Sep 23, 2014
  86. sipa force-pushed on Sep 24, 2014
  87. rebroad commented at 0:27 am on September 29, 2014: contributor
    @sipa Currently, download is stalling using this code because the stall logic is flawed. It is too impatient, and it’s asked about 50 nodes so far for the same block and disconnected all of them because none of them could deliver it quickly enough.
  88. sipa commented at 3:11 am on September 29, 2014: member
    @rebroad Good to know - that definitely sounds like unintended behavior. I’ll have a look at it later, I’m now working on other things.
  89. sipa force-pushed on Sep 29, 2014
  90. rebroad commented at 9:44 am on September 30, 2014: contributor
    @sipa Let me know if you want a copy of my debug.log
  91. sipa force-pushed on Oct 1, 2014
  92. sipa force-pushed on Oct 1, 2014
  93. sipa force-pushed on Oct 1, 2014
  94. sipa force-pushed on Oct 1, 2014
  95. laanwj commented at 1:16 pm on October 1, 2014: member
    Needs rebase
  96. sipa force-pushed on Oct 1, 2014
  97. sipa force-pushed on Oct 1, 2014
  98. sipa commented at 7:40 pm on October 1, 2014: member
    Comparison tool works when run locally, but on Travis it disconnects after the genesis block. @theuni is there any way to look at the log files produced on travis?
  99. sipa commented at 7:41 pm on October 1, 2014: member
    Perhaps we can cat debug.log to the log output in Travis, on comparison tool failure?
  100. theuni commented at 8:51 pm on October 1, 2014: member
    See #5025. We have to be careful about dumping too much, because Travis has an output size ceiling.
  101. sipa force-pushed on Oct 1, 2014
  102. sipa commented at 11:00 pm on October 1, 2014: member
    NPE in pulltester… @TheBlueMatt ?
  103. sipa commented at 1:42 am on October 2, 2014: member

    IT’S GREEEEEEEEEEEN!

    Thanks a lot for the efforts, @TheBlueMatt!

  104. sipa force-pushed on Oct 2, 2014
  105. sipa commented at 2:00 am on October 2, 2014: member
    Moved the comparison tool upgrade to #5027, so this will fail CI again until that is merged.
  106. in src/main.cpp: in f8c97eeead outdated 
    3624+                    // doing this will result in the received block being rejected as an orphan in case it is
3625+                    // not a direct successor.
3626+                    pfrom->PushMessage("getheaders", chainActive.GetLocator(pindexBestHeader), inv.hash);
3627+                    if (chainActive.Tip()->GetBlockTime() > GetAdjustedTime() - Params().TargetSpacing() * 20) {
3628+                        vToFetch.push_back(inv);
3629+                        MarkBlockAsInFlight(pfrom->GetId(), inv.hash);
    rebroad commented at 3:50 am on October 2, 2014:
    Just a niggle, but the naming is a little confusing here - it looks (from the naming) like you’re marking it as in flight, although technically the getdata block hasn’t even occurred yet at this point. This could cause confusion later on down the line in the same way as the recent AlreadyAskedFor naming confusion did recently.
  107. sipa force-pushed on Oct 2, 2014
  108. sipa commented at 4:12 am on October 2, 2014: member
    @rebroad Added a comment that should clarify that.
  109. in src/chain.h: in c6faa4775c outdated 
    119@@ -103,7 +120,8 @@ class CBlockIndex
120     // Note: in a potential headers-first mode, this number cannot be relied upon
121     unsigned int nTx;
122 
123-    // (memory only) Number of transactions in the chain up to and including this block
124+    // (memory only) Number of transactions in the chain up to and including this block.
125+    // This value will be non-zero only iff transactions for this block and all its parents are available.
    Diapolo commented at 10:20 am on October 2, 2014:
    Nit: Typo (iff)
    theuni commented at 5:23 pm on October 2, 2014:
    iff == if and only if
    sipa commented at 6:19 pm on October 2, 2014:
    Indeed; replaced by ‘if and only if’.
    jtimon commented at 6:35 pm on October 2, 2014:
    isn’t “if and only if” more clear?
  110. in src/main.cpp: in c6faa4775c outdated 
    50@@ -50,6 +51,7 @@ bool fTxIndex = false;
51 bool fIsBareMultisigStd = true;
52 unsigned int nCoinCacheSize = 5000;
53 
54+
    Diapolo commented at 10:20 am on October 2, 2014:
    Nit: Unneded
    sipa commented at 6:20 pm on October 2, 2014:
    Fixed.
  111. in src/main.cpp: in c6faa4775c outdated 
    2161+                queue.push_back(it->second);
2162+                range.first++;
2163+                mapBlocksUnlinked.erase(it);
2164+            }
2165+            if (!pblocktree->WriteBlockIndex(CDiskBlockIndex(pindex)))
2166+                return state.Abort(_("Failed to write block index"));
    Diapolo commented at 10:25 am on October 2, 2014:
    Just asking, is it common or good to have these translated?
    sipa commented at 6:21 pm on October 2, 2014:
    I’m just following existing practice in the code here. I have no opinion.
    jtimon commented at 6:36 pm on October 2, 2014:
    I don’t see any reason why we shouldn’t internationalize them.
    gmaxwell commented at 7:17 pm on October 2, 2014:
    Internationalization for rare errors makes them unsearchable (on the internet or in the code base). Internal/rare errors are really not human readable to begin with. If we’re going to do that, we should do it consistently but also assign code numbers for errors.
    laanwj commented at 7:24 pm on October 2, 2014:
    Please only use _() in core for messages passed to the GUI! Otherwise translators spend time translating them for no reason at all. Remember that if you run bitcoind, _() is a no-op.
  112. in src/main.cpp: in c6faa4775c outdated 
    2604-            mapOrphanBlocks.erase(mi->second->hashBlock);
2605-            delete mi->second;
2606+        // Store to disk
2607+        CBlockIndex *pindex = NULL;
2608+        bool ret = AcceptBlock(*pblock, state, &pindex, dbp);
2609+        if (pindex) {
    Diapolo commented at 10:26 am on October 2, 2014:
    Could be if (pindex && pfrom)?
    sipa commented at 6:23 pm on October 2, 2014:
    Fixed.
  113. sipa force-pushed on Oct 2, 2014
  114. in src/main.cpp: in c6faa4775c outdated 
    4392+        if (!state.fSyncStarted && !pto->fClient && fFetch && !fImporting && !fReindex) {
4393+            // Only actively request headers from a single peer, unless we're close to today.
4394+            if (nSyncStarted == 0 || pindexBestHeader->GetBlockTime() > GetAdjustedTime() - 24 * 60 * 60) {
4395+                state.fSyncStarted = true;
4396+                nSyncStarted++;
4397+                CBlockIndex *pindexStart = pindexBestHeader->pprev ? pindexBestHeader->pprev : pindexBestHeader;
    sipa commented at 6:19 pm on October 2, 2014:
    Indeed, this crashes if we only have the genesis block otherwise.
    rebroad commented at 6:16 am on October 4, 2014:
    What happened to the idea of sending a locator that we know the node will have (to avoid it sending thousands of headers we already have)?
    sipa commented at 7:20 am on October 11, 2014:
    Before we know what the peer has, doing that would be asking them for all headers which is much worse.
    rebroad commented at 2:40 pm on October 16, 2014:
    @sipa The change I am proposing results in nodes sending just one header.
  115. sipa force-pushed on Oct 2, 2014
  116. sipa force-pushed on Oct 2, 2014
  117. sipa renamed this:
    Preview: headers-first synchronization
    Headers-first synchronization
    on Oct 2, 2014
  118. rebroad commented at 8:43 am on October 3, 2014: contributor

    @sipa I think the current method of downloading sequential blocks from the same node is fundamentally flawed when there are multiple nodes available because the stall/key node is likely to remain the same node for subsequent blocks. Wouldn’t it be better to skip blocks so that if the node becomes slow it doesn’t slow down the whole window (due to the node at the end of the window remaining the same slow node)?

    Regarding your comment about the stall node - what is the rationale in disconnecting this node? This would just mean that the block being downloaded is partially downloaded and it’s just a guess that another node will be any faster. The only rationale I can see to disconnect a node is if the download is stuck, or going quantifiably slower than it could.

  119. laanwj commented at 10:29 am on October 3, 2014: member

    So great to finally see a green tick! Thanks @TheBlueMatt and @sipa! I say we should move to merge this as soon as possible, as it’s clearly an improvement over the current state.

    it breaks reindexing, which cannot deal with out-of-order blocks on disk yet

    I suppose this is still a blocker?

  120. sipa commented at 3:21 pm on October 3, 2014: member

    Interleaving blocks is an interesting way to improve things. I’m not against that, but at this stage I really just want some form of HF to be merged as it’s so fundamentally better in many ways (independent of the actual download logic improvements). There are various things that could be done, including reducing the window size of slower peers based on how frequently a node comes close to stalling window movement.

    The reason for disconnecting peers which are found by the stalling logic detection is not that they are “slow” by some absolute measure. The fact that they prevent movement of the window means that they are just (much) slower than other peers, and at least for me, disconnecting them seems to very quickly result in selecting fast peers.

  121. gmaxwell commented at 4:06 pm on October 3, 2014: contributor
    Interleaving is pretty pessimal for IO performance of the senders. I don’t think any scheme of interleaving could have more effect than a increase in window size (proportional to the number of peers). Might be interesting to experiment with, but I don’t expect it to be a win all considered.
  122. sipa commented at 4:58 pm on October 3, 2014: member
    @laanwj Yeah, I’ll work on fixing reindexing soon. I wasn’t expecting comparison tool to grow compatible so quickly :)
  123. sipa force-pushed on Oct 3, 2014
  124. rebroad commented at 6:00 am on October 4, 2014: contributor

    @sipa Actually, in some scenarios it would more sense to disconnect all nodes EXCEPT the node currently providing the oldest block (so that the incoming bandwidth isn’t being saturated).

    Also, the code as is seems to have no limit on how many nodes are sync nodes. I’d suggest setting a limit, and then in the situation where the stall node is disconnected (which was providing the oldest block) instead of requesting that from a node that already has many blocks queued (meaning the oldest block won’t get downloaded until all the queued blocks are downloaded from that node first) send the request for blocks (oldest block first) to a node that isn’t currently serving any blocks - that way the oldest block will appear much sooner.

    It would also be good to rotate the nodes that have a break from being downloaded from, so that the speeds of all nodes connected to can be determined and later used for the selection (which node to request a block from) logic.

  125. TheBlueMatt commented at 7:40 am on October 5, 2014: member
    Was mid-review when you rebased, I’m assuming nothing changed in the first commit…in any case, there are some comments on bcf870bf54578d166b5856d8e7978a2072f8a377
  126. TheBlueMatt commented at 7:43 am on October 5, 2014: member
    So I stared at this for a while and couldnt come up with anything clearly broken outside of the few small comments above…still, I’m not as farmiliar with that code as I used to be, so I dont feel comfortable ACKing directly.
  127. sipa force-pushed on Oct 6, 2014
  128. sipa force-pushed on Oct 6, 2014
  129. laanwj referenced this in commit 5346b06d7f on Oct 6, 2014
  130. laanwj referenced this in commit ad6a4bab93 on Oct 6, 2014
  131. laanwj referenced this in commit aedc74dfa6 on Oct 6, 2014
  132. TheBlueMatt commented at 7:21 pm on October 6, 2014: member
    So I’m reasonably happy with everything that isnt -reindex on this one, at least for merge, maybe not release.
  133. laanwj referenced this in commit b911c4da2a on Oct 7, 2014
  134. laanwj referenced this in commit 8377e2807b on Oct 7, 2014
  135. sipa force-pushed on Oct 7, 2014
  136. sipa referenced this in commit ff0367a39b on Oct 7, 2014
  137. sipa force-pushed on Oct 7, 2014
  138. sipa referenced this in commit 575a163912 on Oct 7, 2014
  139. sipa commented at 7:37 pm on October 7, 2014: member
    Included a squashed version of #5057, together with a commit that fixes the skipping of out-of-order blocks on continued reindex.
  140. TheBlueMatt commented at 10:10 pm on October 7, 2014: member
    It seems with the new changes it is no longer possible to concatenate every blk****.dat file into one big bootstrap.dat (with orphans) and import that. I think that is needless feature loss.
  141. sipa commented at 10:26 pm on October 7, 2014: member

    We could have a feature to export the blocks in the block files to a bootstrap.dat… but that is exactly already what the linearize python script does.

    Also, catting the block files together will work for the import functionality in this PR, so it is still internally consistent. Just perhaps not with external application that cannot deal with out-of-order blocks (but that’s what linearize is for).

  142. TheBlueMatt commented at 11:51 pm on October 7, 2014: member
    The way I read it it can only handle non-linear blocks if dbp is non-NULL, which it only is for -reindex.
  143. sipa force-pushed on Oct 8, 2014
  144. sipa commented at 1:17 am on October 8, 2014: member
    @TheBlueMatt Looks like you’re right. That’s unfortunate but not a blocker imho.
  145. TheBlueMatt commented at 1:49 am on October 8, 2014: member
    Well, should be an easy fix so I brought it up.
  146. TheBlueMatt commented at 1:50 am on October 8, 2014: member
    AFAICT dbp is essentially useless there, so adding another one pointing to 0 should be doable?
  147. laanwj commented at 6:12 am on October 8, 2014: member

    It seems with the new changes it is no longer possible to concatenate every blk****.dat file into one big bootstrap.dat (with orphans) and import that.

    I did that intentially. Handling out-of-order blocks in the general case would result in more complicated code than just handling it for reindex. CDiskBlockPos only work inside block files, not to outside files. (the worst case scenario is quite bad! It means being able to handle blocks in any order over multiple files with -loadblock, with possibly non-relevant blocks interspersed)

    To make a bootstrap.dat use linearize.py, I updated it in #5051.

    Anyhow - why bother catting to create a disjointed bootstrap.dat at all? If you want to bootstrap a node from an existing blockchain copy the .blk files to your bitcoin/blocks directory and run -reindex! Its faster too, no (extra) copying.

  148. TheBlueMatt commented at 6:31 am on October 8, 2014: member
    OK, if its more work you’re right its not worth it, I just figured it’d be 0 extra work, so I’m happy if this were merged pending remaining questions. Note that that is not an ACK, I’d like to see this merged and move forward pending enough ACKs from others, but I dont feel comfortable enough with the block import code to ACK this directly.
  149. sipa force-pushed on Oct 8, 2014
  150. sipa commented at 10:14 pm on October 8, 2014: member

    @rebroad to get back on the initial getheaders request and the locator passed to it.

    CBlockLocators contain a list of hashes the peer may know about, including just the last 11 leading up to the block referred to, and then several that go further and further back. This means that it’s always safe to send a locator that is too far ahead - it’s just less efficient.

    The reason for starting with the best header we have, rather than the best we know the peer already has, is because our information about what the peer has may be out of date. In fact, we actually use that getheaders exactly to learn about what they have (that’s the reason for querying one back, so we get something back to learn from). If we don’t learn through any other means, sending a getheaders with the point we do know about them would result in downloading every single header from every single peer, which would be a significant waste of bandwidth.

    I do have a different proposal, but I’m just suggesting it here, and don’t consider it necessary in this pull request: we stop using getheaders to learn about where peers are, and use it only to learn about actual headers ourselves. To compensate, I suggest sending out getblocks for very small sections of headers after learning them ourselves, randomly but infrequently to all peers. The resulting block invs would teach us as much, be more granular and frequent, and be more efficient.

  151. sipa force-pushed on Oct 10, 2014
  152. sipa referenced this in commit 3407604f8f on Oct 10, 2014
  153. rebroad commented at 6:35 am on October 11, 2014: contributor
    @sipa If we send the locator for the block at the advertised block height of the node, then we would get no headers back. If we send the pprev of that block, then we would get 1 header back, the latest header that node has. If for some reason the node has obtained new blocks since we connected to it, we might receive 2 headers back. Either way, I’m not sure I understand your logic that would result in every node sending every header.
  154. in src/main.cpp: in e45522bf9c outdated 
    4494-            LogPrintf("Peer %s is stalling block download, disconnecting\n", state.name);
4495+        if (!pto->fDisconnect && state.nStallingSince && state.nStallingSince < nNow - 1000000 * BLOCK_STALLING_TIMEOUT) {
4496+            // Stalling only triggers when the block download window cannot move. During normal steady state,
4497+            // the download window should be much larger than the to-be-downloaded set of blocks, so disconnection
4498+            // should only happen during initial block download.
4499+            LogPrintf("Peer=%d is stalling block download, disconnecting\n", pto->id);
    rebroad commented at 6:37 am on October 11, 2014:
    This almost always (in my testing) results in slowing down the download of the block chain as almost always the stalling node is currently sending the block we are waiting for, and disconnecting it simply means we’ll have to discard the part of the block already downloaded and request it from another node which is no more likely to be any faster.
    sipa commented at 6:45 am on October 11, 2014:

    Yes, that’s completely expected. The point is that other peers managed to progress past the whole window, while we’re still waiting on this one peer. There is nothing to be gained from keeping it connected, as we’re effectively faster by not downloading from them at all.

    You could argue that we should wait for a short while to give them a chance to finish the block we’re currently fetching, but even in that case, it means wasting time not using bandwidth that other peer could be providing us.

    gmaxwell commented at 6:45 am on October 11, 2014:

    Almost? should be always unless the peer is broken. :) of course, unless its sitting around doing nothing there will be data in flight. A result is that you should loadbalance onto faster peers, so while there is some discovery overhead here it should be finite.

    Further optiimzation could make it somewhat more efficient, e.g. preemptively discontinuing requests and then disconnecting peers that get close to but not quite stalling… but that should probably be future work. (It would add a fair amount of complexity)

  155. gmaxwell commented at 6:42 am on October 11, 2014: contributor
    @rebroad as sipa was saying, your knoweldge of the peer is often out of sync (consider that its by getting blocks/headers from it at all that we learn anything about it). If we only ask for a block it doesn’t have we’re just going to end up pulling like we have nothing in common at all, which is very inefficient.
  156. sipa force-pushed on Oct 11, 2014
  157. sipa force-pushed on Oct 11, 2014
  158. sipa referenced this in commit 0908abfae9 on Oct 11, 2014
  159. laanwj referenced this in commit ff8ed8009a on Oct 13, 2014
  160. Headers-first synchronization 
    Many changes:
* Do not use 'getblocks', but 'getheaders', and use it to build a headers tree.
* Blocks are fetched in parallel from all available outbound peers, using a
  limited moving window. When one peer stalls the movement of the window, it is
  disconnected.
* No more orphan blocks. At all. We only ever request a block for which we have
  verified the headers, and store it to disk immediately. This means that a
  disk-fill attack would require PoW.
* Require protocol version 31800 for every peer (released in december 2010).
* No more syncnode (we sync from everyone we can, though limited to 1 during
  initial *headers* sync).
* Introduce some extra named constants, comments and asserts.
    341735eb8f
  161. RPC additions after headers-first ad6e601712
  162. Remove CheckMinWork, as we always know all parent headers f244c99c96
  163. Improve getheaders (sending) logging 4c93322923
  164. Better logging of stalling 1bcee67ee7
  165. Add height to "Requesting block" debug 1af838b339
  166. Rename setBlockIndexValid to setBlockIndexCandidates e17bd58392
  167. Make -reindex cope with out-of-order blocks 
    Remember out-of-order block headers along with disk positions. This is
likely the simplest and least-impact way to make -reindex work with
headers first.

Based on top of #4468.
    ad96e7ccd9
  168. Skip reindexed blocks individually 
    Instead of skipping to the last reindexed block in each file (which could
jump over processed out-of-order blocks), just skip each already processed
block individually.
    16d5194165
  169. Fix rebuild-chainstate feature and improve its performance 
    Previous refactorings broke the ability to rebuild the chainstate by deleting the chainstate
directory, resulting in an incorrect "Incorrect or no genesis block found" error message. Fix
that.

Also, improve the performance of ActivateBestBlockStep by using the skiplist to only discover
a few potential blocks to connect at a time, instead of all blocks forever - as we likely bail
out after connecting a single one anyway.
    afc32c5eea
  170. sipa force-pushed on Oct 14, 2014
  171. Fix large reorgs e11b2ce4c6
  172. sipa force-pushed on Oct 14, 2014
  173. sipa commented at 11:37 pm on October 14, 2014: member
    I added a commit that AFAICT should allow >2000 deep reorgs. I tried to enable the reorg test in Travis, but that fails due to >4Mbyte of produced log.
  174. laanwj commented at 8:48 am on October 17, 2014: member
    Tested ACK
  175. gmaxwell commented at 8:56 am on October 17, 2014: contributor

    Tested ACK.

    I’ve reviewed this at many points in time and tried many cases (and hit a number of issues which have been fixed). I expect that it still has some remaining bugs, but our behavior without it is almost uselessly bad in the common case… getting some wider usage (in master) will help shake out issues. We know we’re going to have a long cycle of testing in master on this version, so lets get that started.

  176. ghost commented at 9:41 am on October 17, 2014: none
    Seems to function fine on ARMv6.
  177. laanwj merged this on Oct 17, 2014
  178. laanwj closed this on Oct 17, 2014
  179. laanwj referenced this in commit 84d13eef88 on Oct 17, 2014
  180. CodeShark referenced this in commit 9dc5475774 on Oct 20, 2014
  181. in src/main.cpp: in e11b2ce4c6 
    3916+        if (pindexLast)
3917+            UpdateBlockAvailability(pfrom->GetId(), pindexLast->GetBlockHash());
3918+
3919+        if (nCount == MAX_HEADERS_RESULTS && pindexLast) {
3920+            // Headers message had its maximum size; the peer may have more headers.
3921+            // TODO: optimize: if pindexLast is an ancestor of chainActive.Tip or pindexBestHeader, continue
    rebroad commented at 9:10 am on November 6, 2016:
    continue form “there”? there being chainActive.Tip() or pindexBestHeader?
    rebroad commented at 9:29 am on November 6, 2016:
    ah, I get it… ok, coding this…
  182. rebroad referenced this in commit a165da3264 on Nov 6, 2016
  183. rebroad referenced this in commit 53e30e4823 on Nov 6, 2016
  184. rebroad referenced this in commit 6d2b1eab8c on Nov 7, 2016
  185. rebroad referenced this in commit 70f5556b2b on Nov 7, 2016
  186. rebroad referenced this in commit 4885046ec2 on Nov 7, 2016
  187. rebroad referenced this in commit 5c661672ee on Nov 8, 2016
  188. rebroad referenced this in commit fd2088f527 on Nov 9, 2016
  189. rebroad referenced this in commit 78fc33d195 on Nov 11, 2016
  190. rebroad referenced this in commit b58bf95865 on Nov 24, 2016
  191. rebroad referenced this in commit 1739e8bf96 on Nov 24, 2016
  192. rebroad referenced this in commit d9c6b4a0e4 on Nov 29, 2016
  193. rebroad referenced this in commit 3250269cf8 on Nov 30, 2016
  194. rebroad referenced this in commit f33b52d206 on Dec 1, 2016
  195. rebroad referenced this in commit adaad00743 on Dec 1, 2016
  196. rebroad referenced this in commit 7155b5edf1 on Dec 3, 2016
  197. rebroad referenced this in commit 2a9a9d881d on Dec 5, 2016
  198. rebroad referenced this in commit bda0c89dc6 on Dec 6, 2016
  199. rebroad referenced this in commit 8c796e221c on Dec 8, 2016
  200. rebroad referenced this in commit e6357274e6 on Dec 9, 2016
  201. rebroad referenced this in commit e210622998 on Dec 10, 2016
  202. rebroad referenced this in commit 6241ec408c on Dec 11, 2016
  203. rebroad referenced this in commit 2708feae45 on Dec 11, 2016
  204. rebroad referenced this in commit 74eeed39e9 on Dec 11, 2016
  205. rebroad referenced this in commit f4d2f03678 on Dec 12, 2016
  206. rebroad referenced this in commit feefce3752 on Dec 12, 2016
  207. rebroad referenced this in commit 2c57deab0b on Dec 13, 2016
  208. rebroad referenced this in commit 76e66e1c81 on Dec 14, 2016
  209. rebroad referenced this in commit a6380fcd77 on Dec 16, 2016
  210. rebroad referenced this in commit 3592da6544 on Dec 18, 2016
  211. rebroad referenced this in commit d60a203e80 on Dec 20, 2016
  212. rebroad referenced this in commit e7ed9254e9 on Dec 23, 2016
  213. rebroad referenced this in commit cff98c0721 on Dec 23, 2016
  214. rebroad referenced this in commit ea4580e2cc on Dec 23, 2016
  215. rebroad referenced this in commit 9fabdf1801 on Dec 24, 2016
  216. rebroad referenced this in commit c2c61c5b55 on Dec 26, 2016
  217. rebroad referenced this in commit 581cddd6cc on Dec 26, 2016
  218. rebroad referenced this in commit 5f97cc1627 on Dec 26, 2016
  219. rebroad referenced this in commit 44c71d80b6 on Dec 26, 2016
  220. rebroad referenced this in commit 648bba9fd4 on Dec 27, 2016
  221. rebroad referenced this in commit 6a01933548 on Dec 27, 2016
  222. rebroad referenced this in commit a29092d9fe on Dec 30, 2016
  223. rebroad referenced this in commit 6bb4a1840d on Jan 2, 2017
  224. rebroad referenced this in commit 1b9b8d17c5 on Jan 12, 2017
  225. rebroad referenced this in commit 77614793c6 on Jan 17, 2017
  226. rebroad referenced this in commit fe58ab2a71 on Jan 20, 2017
  227. rebroad referenced this in commit 3aa1d66a70 on Jan 20, 2017
  228. rebroad referenced this in commit 83211ba391 on Feb 4, 2017
  229. rebroad referenced this in commit 960dce7ef0 on Feb 4, 2017
  230. rebroad referenced this in commit 33535120bd on Oct 6, 2017
  231. rebroad referenced this in commit 4846e72246 on Oct 7, 2017
  232. rebroad referenced this in commit 97fee0f81a on Oct 11, 2017
  233. rebroad referenced this in commit 8fb5cab0e0 on Oct 11, 2017
  234. rebroad referenced this in commit a63175b3a0 on Mar 2, 2020
  235. rebroad referenced this in commit a25e16b758 on Mar 3, 2020
  236. rebroad referenced this in commit 3c11323615 on Mar 31, 2020
  237. rebroad referenced this in commit bff2342409 on Apr 8, 2020
  238. rebroad referenced this in commit d49d12e227 on Apr 9, 2020
  239. rebroad referenced this in commit d4d89fde3e on Apr 10, 2020
  240. rebroad referenced this in commit 9b6f825ceb on Apr 10, 2020
  241. rebroad referenced this in commit d8a680bb6c on Apr 15, 2020
  242. rebroad referenced this in commit 42083a0170 on Apr 15, 2020
  243. rebroad referenced this in commit 0edfd8650f on Apr 20, 2020
  244. rebroad referenced this in commit 6165d05fe8 on Apr 21, 2020
  245. rebroad referenced this in commit 6f848505d6 on Apr 21, 2020
  246. rebroad referenced this in commit 1f7df5b7cc on Apr 21, 2020
  247. rebroad referenced this in commit 85a11c0c19 on Apr 21, 2020
  248. rebroad referenced this in commit d162f3edda on Apr 21, 2020
  249. rebroad referenced this in commit 7da9d31be7 on Apr 26, 2020
  250. rebroad referenced this in commit a5ea857401 on Apr 27, 2020
  251. rebroad referenced this in commit 98065d319f on Apr 29, 2020
  252. reddink referenced this in commit 4d400db039 on Jul 11, 2020
  253. reddink referenced this in commit d0e737ca14 on Jul 14, 2020
  254. rebroad referenced this in commit 07c47edb6f on Feb 15, 2021
  255. rebroad referenced this in commit 64b31aae1c on Feb 18, 2021
  256. rebroad referenced this in commit d03d226344 on Feb 19, 2021
  257. rebroad referenced this in commit 8ca52d42b1 on Feb 19, 2021
  258. rebroad referenced this in commit db84177fc4 on Mar 28, 2021
  259. DrahtBot locked this on Sep 8, 2021


sipa gmaxwell laanwj rebroad fanquake jgarzik Arnavion BitcoinPullTester theuni Diapolo jtimon TheBlueMatt ghost

Labels
Refactoring P2P

Milestone
0.10.0

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 12:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me