From 96854edd647f808207c23f6a8a21ce382dc59ac1 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Thu, 8 Aug 2024 15:08:57 +1000 Subject: [PATCH 1/4] Handle gossipsub promises gracefully --- .../lighthouse_network/gossipsub/src/behaviour.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs index 0a3b7a9f529..c200566238d 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs @@ -2086,9 +2086,12 @@ where fn apply_iwant_penalties(&mut self) { if let Some((peer_score, ..)) = &mut self.peer_score { for (peer, count) in self.gossip_promises.get_broken_promises() { - peer_score.add_penalty(&peer, count); - if let Some(metrics) = self.metrics.as_mut() { - metrics.register_score_penalty(Penalty::BrokenPromise); + // We do not apply penalties to nodes that have disconnected. + if self.connected_peers.contains_key(&peer) { + peer_score.add_penalty(&peer, count); + if let Some(metrics) = self.metrics.as_mut() { + metrics.register_score_penalty(Penalty::BrokenPromise); + } } } } @@ -2706,8 +2709,8 @@ where for peer_id in recipient_peers { let Some(peer) = self.connected_peers.get_mut(peer_id) else { - tracing::error!(peer = %peer_id, - "Could not IDONTWANT, peer doesn't exist in connected peer list"); + // It can be the case that promises to disconnected peers appear here. In this case + // we simply ignore the peer-id. continue; }; From 03f173a296acf6ce9c03277066a085bbf2e655ea Mon Sep 17 00:00:00 2001 From: Age Manning Date: Thu, 8 Aug 2024 17:35:25 +1000 Subject: [PATCH 2/4] Apply a forgotten patch which sync the fanout with unsubscriptions --- .../gossipsub/src/behaviour.rs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs index c200566238d..485fbd83892 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs @@ -760,7 +760,7 @@ where } } else { tracing::error!(peer_id = %peer_id, - "Could not PUBLISH, peer doesn't exist in connected peer list"); + "Could not send PUBLISH, peer doesn't exist in connected peer list"); } } @@ -1062,7 +1062,7 @@ where }); } else { tracing::error!(peer = %peer_id, - "Could not GRAFT, peer doesn't exist in connected peer list"); + "Could not send GRAFT, peer doesn't exist in connected peer list"); } // If the peer did not previously exist in any mesh, inform the handler @@ -1161,7 +1161,7 @@ where peer.sender.prune(prune); } else { tracing::error!(peer = %peer_id, - "Could not PRUNE, peer doesn't exist in connected peer list"); + "Could not send PRUNE, peer doesn't exist in connected peer list"); } // If the peer did not previously exist in any mesh, inform the handler @@ -1340,7 +1340,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not IWANT, peer doesn't exist in connected peer list"); + "Could not send IWANT, peer doesn't exist in connected peer list"); } } tracing::trace!(peer=%peer_id, "Completed IHAVE handling for peer"); @@ -1363,7 +1363,7 @@ where for id in iwant_msgs { // If we have it and the IHAVE count is not above the threshold, - // foward the message. + // forward the message. if let Some((msg, count)) = self .mcache .get_with_iwant_counts(&id, peer_id) @@ -1403,7 +1403,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not IWANT, peer doesn't exist in connected peer list"); + "Could not send IWANT, peer doesn't exist in connected peer list"); } } } @@ -2043,8 +2043,11 @@ where } } - // remove unsubscribed peers from the mesh if it exists + // remove unsubscribed peers from the mesh and fanout if they exist there for (peer_id, topic_hash) in unsubscribed_peers { + self.fanout + .get_mut(&topic_hash) + .map(|peers| peers.remove(&peer_id)); self.remove_peer_from_mesh(&peer_id, &topic_hash, None, false, Churn::Unsub); } @@ -2068,7 +2071,7 @@ where } } else { tracing::error!(peer = %propagation_source, - "Could not GRAFT, peer doesn't exist in connected peer list"); + "Could not send GRAFT, peer doesn't exist in connected peer list"); } // Notify the application of the subscriptions @@ -2586,7 +2589,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not IHAVE, peer doesn't exist in connected peer list"); + "Could not send IHAVE, peer doesn't exist in connected peer list"); } } } @@ -2672,7 +2675,7 @@ where peer.sender.prune(prune); } else { tracing::error!(peer = %peer_id, - "Could not PRUNE, peer doesn't exist in connected peer list"); + "Could not send PRUNE, peer doesn't exist in connected peer list"); } // inform the handler @@ -2975,7 +2978,7 @@ where } } else { tracing::error!(peer = %peer_id, - "Could not SUBSCRIBE, peer doesn't exist in connected peer list"); + "Could not send SUBSCRIBE, peer doesn't exist in connected peer list"); } } From e2764d6a10ca6037144344d76e4e926a907e8676 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Fri, 9 Aug 2024 09:16:52 +1000 Subject: [PATCH 3/4] Update beacon_node/lighthouse_network/gossipsub/src/behaviour.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: João Oliveira --- beacon_node/lighthouse_network/gossipsub/src/behaviour.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs index 485fbd83892..1627777f845 100644 --- a/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs +++ b/beacon_node/lighthouse_network/gossipsub/src/behaviour.rs @@ -2043,7 +2043,7 @@ where } } - // remove unsubscribed peers from the mesh and fanout if they exist there + // remove unsubscribed peers from the mesh and fanout if they exist there. for (peer_id, topic_hash) in unsubscribed_peers { self.fanout .get_mut(&topic_hash) From 24217fa3613d86bab041e8108fd2bf829bef03d2 Mon Sep 17 00:00:00 2001 From: Age Manning Date: Tue, 3 Sep 2024 14:32:17 +1000 Subject: [PATCH 4/4] Add changelog entry --- beacon_node/lighthouse_network/gossipsub/CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md b/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md index 7ec10af741e..b93e189e2c8 100644 --- a/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md +++ b/beacon_node/lighthouse_network/gossipsub/CHANGELOG.md @@ -1,5 +1,8 @@ ## 0.5 Sigma Prime fork +- Correct state inconsistencies with the mesh and connected peers due to the fanout mapping. + See [PR 6244](https://github.com/sigp/lighthouse/pull/6244) + - Implement IDONTWANT messages as per [spec](https://github.com/libp2p/specs/pull/548). See [PR 5422](https://github.com/sigp/lighthouse/pull/5422)