From 72c40a03feeffe5b1c507c11c73e93d8f0e1d22c Mon Sep 17 00:00:00 2001 From: iho Date: Thu, 9 Jul 2026 11:41:03 +0300 Subject: [PATCH] fix: update peer difficulty on header/block receipt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PeerLiveInfo height/total_difficulty was only refreshed on ping/pong, so a peer that just sent a new tip header could look stale for up to ~10s. Refresh live info (if work increased) when we accept a header, compact block, or full block — including already-known duplicates that still advertise that peer has the tip. Closes #3167 --- p2p/src/types.rs | 74 ++++++++++++++++++++++++++++++++++ servers/src/common/adapters.rs | 34 ++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/p2p/src/types.rs b/p2p/src/types.rs index 03acd8ecdd..44cee70d1c 100644 --- a/p2p/src/types.rs +++ b/p2p/src/types.rs @@ -641,6 +641,25 @@ impl PeerInfo { live_info.total_difficulty = total_difficulty; live_info.last_seen = Utc::now() } + + /// Update height/difficulty only when the peer advertises *more* work + /// (higher total_difficulty, or same difficulty at a greater height). + /// Used when a peer sends us a header/block/compact block so we do not wait + /// for the next ping/pong (up to ~10s) to reflect their chain tip. + pub fn update_if_better(&self, height: u64, total_difficulty: Difficulty) { + let mut live_info = self.live_info.write(); + let better = total_difficulty > live_info.total_difficulty + || (total_difficulty == live_info.total_difficulty && height > live_info.height); + if !better { + return; + } + if total_difficulty != live_info.total_difficulty { + live_info.stuck_detector = Utc::now(); + } + live_info.height = height; + live_info.total_difficulty = total_difficulty; + live_info.last_seen = Utc::now() + } } /// Flatten out a PeerInfo and nested PeerLiveInfo (taking a read lock on it) @@ -873,6 +892,8 @@ pub trait NetAdapter: ChainAdapter { fn peer_addrs_received(&self, _: Vec); /// Heard total_difficulty from a connected peer (via ping/pong). + /// Header/block/compact-block receipts also update peer live info directly + /// via [`PeerInfo::update_if_better`] so tip tracking is not delayed until ping. fn peer_difficulty(&self, _: PeerAddr, _: Difficulty, _: u64); /// Is this peer currently banned? @@ -894,3 +915,56 @@ pub struct AttachmentUpdate { pub left: usize, pub meta: Arc, } + +#[cfg(test)] +mod test { + use super::*; + use crate::core::ser::ProtocolVersion; + use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + + fn test_peer_info(difficulty: Difficulty) -> PeerInfo { + PeerInfo { + capabilities: Capabilities::default(), + user_agent: "test".into(), + version: ProtocolVersion(1), + addr: PeerAddr(SocketAddr::new(IpAddr::V4(Ipv4Addr::LOCALHOST), 3414)), + direction: Direction::Outbound, + live_info: Arc::new(RwLock::new(PeerLiveInfo::new(difficulty))), + } + } + + #[test] + fn update_if_better_increases_work() { + let peer = test_peer_info(Difficulty::from_num(10)); + assert_eq!(peer.total_difficulty(), Difficulty::from_num(10)); + assert_eq!(peer.height(), 0); + + // lower difficulty is ignored + peer.update_if_better(5, Difficulty::from_num(5)); + assert_eq!(peer.total_difficulty(), Difficulty::from_num(10)); + assert_eq!(peer.height(), 0); + + // equal difficulty, equal height ignored + peer.update_if_better(0, Difficulty::from_num(10)); + assert_eq!(peer.height(), 0); + + // equal difficulty, higher height accepted + peer.update_if_better(3, Difficulty::from_num(10)); + assert_eq!(peer.total_difficulty(), Difficulty::from_num(10)); + assert_eq!(peer.height(), 3); + + // equal difficulty, lower height ignored + peer.update_if_better(1, Difficulty::from_num(10)); + assert_eq!(peer.height(), 3); + + // higher difficulty accepted + peer.update_if_better(4, Difficulty::from_num(20)); + assert_eq!(peer.total_difficulty(), Difficulty::from_num(20)); + assert_eq!(peer.height(), 4); + + // absolute update still overwrites (ping/pong) + peer.update(2, Difficulty::from_num(15)); + assert_eq!(peer.total_difficulty(), Difficulty::from_num(15)); + assert_eq!(peer.height(), 2); + } +} diff --git a/servers/src/common/adapters.rs b/servers/src/common/adapters.rs index 5bdcb5301b..58c388ac18 100644 --- a/servers/src/common/adapters.rs +++ b/servers/src/common/adapters.rs @@ -150,7 +150,9 @@ where peer_info: &PeerInfo, opts: chain::Options, ) -> Result { + // Peer is advertising this work even if we already know the block. if self.chain().is_known(&b.header).is_err() { + Self::update_peer_from_header(peer_info, &b.header); return Ok(true); } @@ -172,7 +174,9 @@ where peer_info: &PeerInfo, ) -> Result { // No need to process this compact block if we have previously accepted the _full block_. + // Still refresh peer difficulty/height from the header they sent. if self.chain().is_known(&cb.header).is_err() { + Self::update_peer_from_header(peer_info, &cb.header); return Ok(true); } @@ -217,8 +221,13 @@ where .process_block_header(&cb.header, chain::Options::NONE) { debug!("Invalid compact block header {}: {:?}", cb_hash, e); + // Already-known / non-ban errors: still refresh peer work if we have the header. + if !e.is_bad_data() && self.chain().get_block_header(&cb.header.hash()).is_ok() { + Self::update_peer_from_header(peer_info, &cb.header); + } return Ok(!e.is_bad_data()); } + Self::update_peer_from_header(peer_info, &cb.header); let (txs, missing_short_ids) = { self.tx_pool @@ -283,7 +292,9 @@ where peer_info: &PeerInfo, ) -> Result { // No need to process this header if we have previously accepted the _full block_. + // Still update peer live info — sending the header advertises that work. if self.chain().block_exists(bh.hash())? { + Self::update_peer_from_header(peer_info, &bh); return Ok(true); } if !self.sync_state.is_syncing() { @@ -301,6 +312,11 @@ where if e.is_bad_data() { return Ok(false); } else { + // Header may still be known on the header chain (e.g. already seen during sync). + // Refresh peer work when the header is not bad data. + if self.chain().get_block_header(&bh.hash()).is_ok() { + Self::update_peer_from_header(peer_info, &bh); + } // we got an error when trying to process the block header // but nothing serious enough to need to ban the peer upstream return Err(e); @@ -308,6 +324,7 @@ where } // we have successfully processed a block header + Self::update_peer_from_header(peer_info, &bh); // so we can go request the block itself self.request_compact_block(&bh, peer_info); @@ -350,6 +367,9 @@ where if let Some(sync_head) = sync_head { self.sync_state.update_header_sync(sync_head); } + if let Some(best) = bhs.iter().max_by_key(|h| h.total_difficulty()) { + Self::update_peer_from_header(peer_info, best); + } Ok(true) } Err(e) => { @@ -882,6 +902,12 @@ where None } + /// Refresh peer height/difficulty from a header they sent us (header-first + /// gossip, compact block, or full block). Only increases advertised work. + fn update_peer_from_header(peer_info: &PeerInfo, header: &BlockHeader) { + peer_info.update_if_better(header.height, header.total_difficulty()); + } + // pushing the new block through the chain pipeline // remembering to reset the head if we have a bad block fn process_block( @@ -902,10 +928,12 @@ where } let bhash = b.hash(); + let header = b.header.clone(); let previous = self.chain().get_previous_header(&b.header); match self.chain().process_block(b, opts) { Ok(_) => { + Self::update_peer_from_header(peer_info, &header); self.validate_chain(bhash); self.check_compact(); Ok(true) @@ -917,6 +945,8 @@ where Err(e) => { match e { chain::Error::Orphan => { + // Peer has this block (orphan to us); still advertise their work. + Self::update_peer_from_header(peer_info, &header); if let Ok(previous) = previous { // make sure we did not miss the parent block if !self.chain().is_orphan(&previous.hash()) @@ -930,6 +960,10 @@ where } _ => { debug!("process_block: block {} refused by chain: {}", bhash, e); + // Already known / unfit: refresh peer only if we have this header. + if self.chain().get_block_header(&header.hash()).is_ok() { + Self::update_peer_from_header(peer_info, &header); + } Ok(true) } }