Skip to content
New issue

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

Sign up for GitHub

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

Jump to bottom

Fix duplicate headers download in initial sync #1589

Merged
merged 3 commits into from
Sep 4, 2017
Merged

Fix duplicate headers download in initial sync #1589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c3d6f54
Fix duplicate headers download in initial sync
codablock Aug 24, 2017
fac0a6b
Introduce DelayGetHeadersTime chain param and fix tests
codablock Sep 1, 2017
6182c28
Disconnect peers which are not catched up
codablock Sep 3, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions src/chainparams.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ class CMainParams : public CChainParams {
vAlertPubKey = ParseHex("048240a8748a80a286b270ba126705ced4f2ce5a7847b3610ea3c06513150dade2a8512ed5ea86320824683fc0818f0ac019214973e677acd1244f6d0571fc5103");
nDefaultPort = 9999;
nMaxTipAge = 6 * 60 * 60; // ~144 blocks behind -> 2 x fork detection time, was 24 * 60 * 60 in bitcoin
nDelayGetHeadersTime = 24 * 60 * 60;
nPruneAfterHeight = 100000;

genesis = CreateGenesisBlock(1390095618, 28917698, 0x1e0ffff0, 1, 50 * COIN);
Expand Down Expand Up @@ -250,6 +251,7 @@ class CTestNetParams : public CChainParams {
vAlertPubKey = ParseHex("04517d8a699cb43d3938d7b24faaff7cda448ca4ea267723ba614784de661949bf632d6304316b244646dea079735b9a6fc4af804efb4752075b9fe2245e14e412");
nDefaultPort = 19999;
nMaxTipAge = 0x7fffffff; // allow mining on top of old blocks for testnet
nDelayGetHeadersTime = 24 * 60 * 60;
nPruneAfterHeight = 1000;

genesis = CreateGenesisBlock(1390666206UL, 3861367235UL, 0x1e0ffff0, 1, 50 * COIN);
Expand Down Expand Up @@ -359,6 +361,7 @@ class CRegTestParams : public CChainParams {
pchMessageStart[2] = 0xb7;
pchMessageStart[3] = 0xdc;
nMaxTipAge = 6 * 60 * 60; // ~144 blocks behind -> 2 x fork detection time, was 24 * 60 * 60 in bitcoin
nDelayGetHeadersTime = 0; // never delay GETHEADERS in regtests
nDefaultPort = 19994;
nPruneAfterHeight = 1000;

Expand Down
2 changes: 2 additions & 0 deletions src/chainparams.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class CChainParams
/** Policy: Filter transactions that do not match well-defined patterns */
bool RequireStandard() const { return fRequireStandard; }
int64_t MaxTipAge() const { return nMaxTipAge; }
int64_t DelayGetHeadersTime() const { return nDelayGetHeadersTime; }
uint64_t PruneAfterHeight() const { return nPruneAfterHeight; }
/** Make miner stop after a block is found. In RPC, don't return until nGenProcLimit blocks are generated */
bool MineBlocksOnDemand() const { return fMineBlocksOnDemand; }
Expand All @@ -89,6 +90,7 @@ class CChainParams
std::vector<unsigned char> vAlertPubKey;
int nDefaultPort;
long nMaxTipAge;
int64_t nDelayGetHeadersTime;
uint64_t nPruneAfterHeight;
std::vector<CDNSSeedData> vSeeds;
std::vector<unsigned char> base58Prefixes[MAX_BASE58_TYPES];
Expand Down
9 changes: 9 additions & 0 deletions src/net.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -742,6 +742,9 @@ class CNode
// Used for headers announcements - unfiltered blocks to relay
// Also protected by cs_inventory
std::vector<uint256> vBlockHashesToAnnounce;
// Blocks received by INV while headers chain was too far behind. These are used to delay GETHEADERS messages
// Also protected by cs_inventory
std::vector<uint256> vBlockHashesFromINV;

// Block and TXN accept times
std::atomic<int64_t> nLastBlockTime;
Expand Down Expand Up @@ -875,6 +878,12 @@ class CNode
vBlockHashesToAnnounce.push_back(hash);
}

void PushBlockHashFromINV(const uint256 &hash)
{
LOCK(cs_inventory);
vBlockHashesFromINV.push_back(hash);
}

void AskFor(const CInv& inv);

void CloseSocketDisconnect();
Expand Down
78 changes: 55 additions & 23 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1380,24 +1380,35 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
if (inv.type == MSG_BLOCK) {
UpdateBlockAvailability(pfrom->GetId(), inv.hash);
if (!fAlreadyHave && !fImporting && !fReindex && !mapBlocksInFlight.count(inv.hash)) {
// First request the headers preceding the announced block. In the normal fully-synced
// case where a new block is announced that succeeds the current tip (no reorganization),
// there are no such headers.
// Secondly, and only when we are close to being synced, we request the announced block directly,
// to avoid an extra round-trip. Note that we must *first* ask for the headers, so by the
// time the block arrives, the header chain leading up to it is already validated. Not
// doing this will result in the received block being rejected as an orphan in case it is
// not a direct successor.
connman.PushMessage(pfrom, NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), inv.hash);
CNodeState *nodestate = State(pfrom->GetId());
if (CanDirectFetch(chainparams.GetConsensus()) &&
nodestate->nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
vToFetch.push_back(inv);
// Mark block as in flight already, even though the actual "getdata" message only goes out
// later (within the same cs_main lock, though).
MarkBlockAsInFlight(pfrom->GetId(), inv.hash, chainparams.GetConsensus());
if (chainparams.DelayGetHeadersTime() != 0 && pindexBestHeader->GetBlockTime() < GetAdjustedTime() - chainparams.DelayGetHeadersTime()) {
// We are pretty far from being completely synced at the moment. If we would initiate a new
// chain of GETHEADERS/HEADERS now, we may end up downnloading the full chain from multiple
// peers at the same time, slowing down the initial sync. At the same time, we don't know
// if the peer we got this INV from may have a chain we don't know about yet, so we HAVE TO
// send a GETHEADERS message at some point in time. This is delayed to later in SendMessages
// when the headers chain has catched up enough.
LogPrint("net", "delaying getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, inv.hash.ToString(), pfrom->id);
pfrom->PushBlockHashFromINV(inv.hash);
} else {
// First request the headers preceding the announced block. In the normal fully-synced
// case where a new block is announced that succeeds the current tip (no reorganization),
// there are no such headers.
// Secondly, and only when we are close to being synced, we request the announced block directly,
// to avoid an extra round-trip. Note that we must *first* ask for the headers, so by the
// time the block arrives, the header chain leading up to it is already validated. Not
// doing this will result in the received block being rejected as an orphan in case it is
// not a direct successor.
connman.PushMessage(pfrom, NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), inv.hash);
CNodeState *nodestate = State(pfrom->GetId());
if (CanDirectFetch(chainparams.GetConsensus()) &&
nodestate->nBlocksInFlight < MAX_BLOCKS_IN_TRANSIT_PER_PEER) {
vToFetch.push_back(inv);
// Mark block as in flight already, even though the actual "getdata" message only goes out
// later (within the same cs_main lock, though).
MarkBlockAsInFlight(pfrom->GetId(), inv.hash, chainparams.GetConsensus());
}
LogPrint("net", "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, inv.hash.ToString(), pfrom->id);
}
LogPrint("net", "getheaders (%d) %s to peer=%d\n", pindexBestHeader->nHeight, inv.hash.ToString(), pfrom->id);
}
}
else
Expand Down Expand Up @@ -1764,11 +1775,6 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
ReadCompactSize(vRecv); // ignore tx count; assume it is 0.
}

if (nCount == 0) {
// Nothing interesting. Stop asking this peers for more headers.
return true;
}

CBlockIndex *pindexLast = NULL;
{
LOCK(cs_main);
Expand Down Expand Up @@ -1805,6 +1811,20 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
// from there instead.
LogPrint("net", "more getheaders (%d) to end to peer=%d (startheight:%d)\n", pindexLast->nHeight, pfrom->id, pfrom->nStartingHeight);
connman.PushMessage(pfrom, NetMsgType::GETHEADERS, chainActive.GetLocator(pindexLast), uint256());
} else {
if (chainparams.DelayGetHeadersTime() != 0 && pindexBestHeader->GetBlockTime() < GetAdjustedTime() - chainparams.DelayGetHeadersTime()) {
// peer has sent us a HEADERS message below maximum size and we are still quite far from being fully
// synced, this means we probably got a bad peer for initial sync and need to continue with another one.
// By disconnecting we force to start a new iteration of initial headers sync in SendMessages
// TODO should we handle whitelisted peers here as we do in headers sync timeout handling?
pfrom->fDisconnect = true;
return error("detected bad peer for initial headers sync, disconnecting %d", pfrom->id);
}

if (nCount == 0) {
// Nothing interesting. Stop asking this peers for more headers.
return true;
}
}

bool fCanDirectFetch = CanDirectFetch(chainparams.GetConsensus());
Expand Down Expand Up @@ -2274,7 +2294,8 @@ bool ProcessMessages(CNode* pfrom, CConnman& connman, std::atomic<bool>& interru

bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsgProc)
{
const Consensus::Params& consensusParams = Params().GetConsensus();
const CChainParams chainParams = Params();
const Consensus::Params& consensusParams = chainParams.GetConsensus();
{
// Don't send anything until the version handshake is complete
if (!pto->fSuccessfullyConnected || pto->fDisconnect)
Expand Down Expand Up @@ -2391,6 +2412,17 @@ bool SendMessages(CNode* pto, CConnman& connman, std::atomic<bool>& interruptMsg
}
}

if (chainParams.DelayGetHeadersTime() != 0 && pindexBestHeader->GetBlockTime() >= GetAdjustedTime() - chainParams.DelayGetHeadersTime()) {
// Headers chain has catched up enough so we can send out GETHEADER messages which were initially meant to
// be sent directly after INV was received
LOCK(pto->cs_inventory);
BOOST_FOREACH(const uint256 &hash, pto->vBlockHashesFromINV) {
LogPrint("net", "process delayed getheaders (%d) to peer=%d\n", pindexBestHeader->nHeight, pto->id);
connman.PushMessage(pto, NetMsgType::GETHEADERS, chainActive.GetLocator(pindexBestHeader), hash);
}
pto->vBlockHashesFromINV.clear();
}

// Resend wallet transactions that haven't gotten in a block yet
// Except during reindex, importing and IBD, when old wallet
// transactions become unconfirmed and spams other nodes.
Expand Down