diff -uNr a/bitcoin/manifest b/bitcoin/manifest --- a/bitcoin/manifest cfb0869447490ccb6a1b6a86944ad321bb6e71732e371075f89034d8ec25496edff2b465bb943db8692ebb3201d56a2015b6de5dc7eca6e6e662aaf4eb3fa492 +++ b/bitcoin/manifest a1eae616970b1886043d5fd2d8477d5f02f08c97d0ae65c3537e4267c1981470bdb501a806b3c05941a6fdf2b42d73188dc743c47578254b5b2750d0261d721b @@ -44,3 +44,4 @@ 732505 bitcoin_getblockindex_etc_corrected jfw Add 'getblockindex' RPC command to view CBlockIndex objects as found in the memory block index and ultimately the database. Also corrects the InvalidChainFound log message for consistency with the safe mode warning changed in bitcoin_help_tuneups, and adds 'eatblock' to the safe mode command whitelist (since it's equivalent to receiving blocks from the network, which is not restricted in safe mode). (Prior patch version missed the newline in the log message.) 732509 bitcoin_drop_online_build jfw Remove code and comments pertaining to the ONLINE=1 option which fetched dependency libs from deedbot, and upgrade their manifest hashes from sha512sum to keksum format. 735223 bitcoin_response_size_limits_1 jfw Simple adjustments to prepare the code for more significant ones to improve response size limiting at the source. CBlockLocator::GetBlockIndex never returns NULL unless something is very wrong. Don't log when the getblocks response stops at the requested hash: it's uninteresting since that hash was already logged just before, and this allows simplifying loop bounds. Differentiate getheaders implementation between the null locator (single block) case and the normal loop. Always log starting heights for getblocks/getheaders rather than punting to -1 just because a non-null pointer wasn't immediately available. +735223 bitcoin_response_size_limits_2 jfw getblocks: switch from nonsensical block count based limit (which didn't really limit at all) and heavy-weight yet still inexact bytes based limit to a simple conservative bound. getheaders: switch from nonsensical limit (which didn't really limit at all) to a sending buffer size derived limit to prevent flooding or at least give peer the chance to do so. diff -uNr a/bitcoin/src/main.cpp b/bitcoin/src/main.cpp --- a/bitcoin/src/main.cpp ea612c8910efd8e86d1a9eaa3668de9c8d586d495db3567b8e4115a070c3c6e8f07e60797411f0b511e4290877fa828c181bb730093812cc05f9db4620868345 +++ b/bitcoin/src/main.cpp 00dc0778ec8886372de6a2f33a914f4e5d0360d7bb690b242f770f259ac562bfd3f663c9bd554f911a0b935522ee1377ffd0eaef0c65d525ae17ebd60a9a23ed @@ -1965,20 +1965,19 @@ } unsigned int nStartHeight = pindex->nHeight + 1; pindex = pindex->pnext; - int nLimit = 500 + locator.GetDistanceBack(); - unsigned int nBytes = 0; + // Set a conservative limit to avoid flooding the sending buffer and ensure hashContinue is reached when processing the anticipated followup getdata, yet keeping the processing and I/O for this response low. Think of it as a pipeline depth for the download process. + unsigned int nLimit = SendBufferSize() / MAX_BLOCK_SIZE / 2; + if (nLimit == 0) nLimit = 1; printf("getblocks %d to %s limit %d\n", nStartHeight, hashStop.ToString().c_str(), nLimit); for (; pindex && pindex->GetBlockHash() != hashStop; pindex = pindex->pnext) { pfrom->PushInventory(CInv(MSG_BLOCK, pindex->GetBlockHash())); - CBlock block; - block.ReadFromDisk(pindex, true); - nBytes += block.GetSerializeSize(SER_NETWORK); - if (--nLimit <= 0 || nBytes >= SendBufferSize()/2) + if (--nLimit == 0) { // When this block is requested, we'll send an inv that'll make them // getblocks the next batch of inventory. - printf(" getblocks stopping at limit %d %s (%u bytes)\n", pindex->nHeight, pindex->GetBlockHash().ToString().c_str(), nBytes); + printf(" getblocks stopping at limit %d %s\n", pindex->nHeight, pindex->GetBlockHash().ToString().c_str()); + // TODO somehow move this state tracking to the recipient side because there's no guarantee that it sends a getdata for the continuation block. pfrom->hashContinue = pindex->GetBlockHash(); break; } @@ -2014,13 +2013,15 @@ return false; } unsigned int nStartHeight = pindex->nHeight + 1; + // Avoid flooding the sending buffer, with room to spare for overhead and other requests that may be pending. + unsigned int nLimit = SendBufferSize() / pindex->GetBlockHeader().GetSerializeSize() / 2; + if (nLimit == 0) nLimit = 1; pindex = pindex->pnext; - int nLimit = 2000 + locator.GetDistanceBack(); printf("getheaders %d to %s limit %d\n", nStartHeight, hashStop.ToString().c_str(), nLimit); for (; pindex; pindex = pindex->pnext) { vHeaders.push_back(pindex->GetBlockHeader()); - if (--nLimit <= 0 || pindex->GetBlockHash() == hashStop) + if (--nLimit == 0 || pindex->GetBlockHash() == hashStop) break; } }