When a BIP152 HB-mode peer is in the least preferred position and disconnects, they will not be by ForNode on the next loop. They will continue to sit in that position and prevent deactivating HB mode for peers that are still connected.
There is no reason for them to stay in the list if already gone, so drop the first element unconditionally if there are too many.
Fixes issue #9163.
When a BIP152 HB-mode peer is in the least preferred position and
disconnects, they will not be by ForNode on the next loop. They
will continue to sit in that position and prevent deactivating
HB mode for peers that are still connected.
There is no reason for them to stay in the list if already gone,
so drop the first element unconditionally if there are too many.
Fixes issue #9163.
528@@ -529,12 +529,11 @@ void MaybeSetPeerAsAnnouncingHeaderAndIDs(const CNodeState* nodestate, CNode* pf
529 if (lNodesAnnouncingHeaderAndIDs.size() >= 3) {
530 // As per BIP152, we only get 3 of our peers to announce
531 // blocks using compact encodings.
532- bool found = connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [&connman, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion](CNode* pnodeStop){
533+ connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [&connman, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion](CNode* pnodeStop){
534 connman.PushMessage(pnodeStop, NetMsgType::SENDCMPCT, fAnnounceUsingCMPCTBLOCK, nCMPCTBLOCKVersion);
535 return true;
Why not just erase the id from lNodesAnnouncingHeaderAndIDs in FinalizeNode()?
I thought about that, but it’s actually irrelevant until a new peer sends a block in any case… and would still need this code regardless, to care care of peers that are removed but not finalized yet.
It might make more sense in the future if we increase the list beyond three in order to have some ‘spares’ that can be activated when a HB selected peer goes offline (so when a HB peer goes offline we’d activate one of the spares)… certainly not something to do for a fix that needs to get backported though. :)
