Skip to content

Log new headers #27278

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

Merged
merged 2 commits into from
Mar 21, 2023
Merged

Log new headers #27278

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e5ce857
log: net: new header over cmpctblock
jamesob Mar 19, 2023
2c3a90f
log: on new valid header
jamesob Mar 19, 2023
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
16 changes: 11 additions & 5 deletions src/net_processing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4190,6 +4190,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
vRecv >> cmpctblock;

bool received_new_header = false;
const auto blockhash = cmpctblock.header.GetHash();

{
LOCK(cs_main);
Expand All @@ -4207,7 +4208,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
return;
}

if (!m_chainman.m_blockman.LookupBlockIndex(cmpctblock.header.GetHash())) {
if (!m_chainman.m_blockman.LookupBlockIndex(blockhash)) {
received_new_header = true;
}
}
Expand All @@ -4221,6 +4222,11 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
}
}

if (received_new_header) {
LogPrintfCategory(BCLog::NET, "Saw new cmpctblock header hash=%s peer=%d\n",
blockhash.ToString(), pfrom.GetId());
}

// When we succeed in decoding a block's txids from a cmpctblock
// message we typically jump to the BLOCKTXN handling code, with a
// dummy (empty) BLOCKTXN message, to re-use the logic there in
Expand Down Expand Up @@ -4263,7 +4269,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// We requested this block for some reason, but our mempool will probably be useless
// so we just grab the block via normal getdata
std::vector<CInv> vInv(1);
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash());
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash);
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv));
}
return;
Expand Down Expand Up @@ -4299,7 +4305,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
} else if (status == READ_STATUS_FAILED) {
// Duplicate txindexes, the block is now in-flight, so just request it
std::vector<CInv> vInv(1);
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash());
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash);
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv));
return;
}
Expand All @@ -4312,7 +4318,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
if (req.indexes.empty()) {
// Dirty hack to jump to BLOCKTXN code (TODO: move message handling into their own functions)
BlockTransactions txn;
txn.blockhash = cmpctblock.header.GetHash();
txn.blockhash = blockhash;
blockTxnMsg << txn;
fProcessBLOCKTXN = true;
} else {
Expand Down Expand Up @@ -4342,7 +4348,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
// We requested this block, but its far into the future, so our
// mempool will probably be useless - request the block normally
std::vector<CInv> vInv(1);
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash());
vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), blockhash);
m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv));
return;
} else {
Expand Down
17 changes: 17 additions & 0 deletions src/validation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3832,6 +3832,23 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida
if (ppindex)
*ppindex = pindex;

// Since this is the earliest point at which we have determined that a
// header is both new and valid, log here.
//
// These messages are valuable for detecting potential selfish mining behavior;
// if multiple displacing headers are seen near simultaneously across many
// nodes in the network, this might be an indication of selfish mining. Having
// this log by default when not in IBD ensures broad availability of this data
// in case investigation is merited.
const auto msg = strprintf(
"Saw new header hash=%s height=%d", hash.ToString(), pindex->nHeight);
Comment on lines +3843 to +3844
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leaving this style nit in case someone ends up re-touching this in a follow-up, because this makes my eyes burn 😭

Suggested change
const auto msg = strprintf(
"Saw new header hash=%s height=%d", hash.ToString(), pindex->nHeight);
const auto msg = strprintf(
"Saw new header hash=%s height=%d", hash.ToString(), pindex->nHeight
);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@josibake Our clang-format wouldn't agree. It would revert your diff to what was merged (and propose this change instead).

@@ -4224,7 +4224,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
 
         if (received_new_header) {
             LogPrintfCategory(BCLog::NET, "Saw new cmpctblock header hash=%s peer=%d\n",
-                blockhash.ToString(), pfrom.GetId());
+                              blockhash.ToString(), pfrom.GetId());
         }


if (ActiveChainstate().IsInitialBlockDownload()) {
LogPrintLevel(BCLog::VALIDATION, BCLog::Level::Debug, "%s\n", msg);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This creates a nice flood of messages when running with -debug=validation right after our headers presync succeeds. But that's probably fine if someone is actively debugging IBD.

} else {
LogPrintf("%s\n", msg);
}

return true;
}

Expand Down