Ban peers after repeated invalid channel messages - #1532
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
chenyukang
left a comment
chenyukang
left a comment
There was a problem hiding this comment.
Inline notes for the issues I mentioned in the earlier review summary.
| } | ||
|
|
||
| self.peer_session_map.remove(&pubkey); | ||
| self.invalid_channel_msg_count.remove(&pubkey); |
There was a problem hiding this comment.
[P1] Preserve invalid-message strikes across voluntary reconnects
This removes the per-peer counter on every normal disconnect, but the ban is only created when the counter becomes greater than 20. A peer can therefore send exactly 20 invalid-channel messages, disconnect voluntarily, reconnect with a fresh counter, and repeat indefinitely without ever entering banned_peers. Please retain the violation state across disconnects until an explicit expiry window, and add a regression test covering 20 invalid messages, disconnect, reconnect, then one more invalid message.
| return; | ||
| } | ||
| // Ban expired, clean up | ||
| self.banned_peers.remove(&remote_pubkey); |
There was a problem hiding this comment.
[P2] Expire temporary bans independently of an inbound reconnect
Expired entries are removed only when the same peer reaches on_peer_connected. Banning also inserts the peer into requested_disconnect_peers, so all local reconnect paths skip it. If the remote peer does not initiate another connection after the ten-minute deadline, no code removes either entry or resumes local reconnects: the temporary ban effectively becomes permanent for outbound connectivity, and peers that never return leave stale entries in both maps indefinitely. Please schedule expiry or purge expired bans during maintenance, remove the requested-disconnect marker, and resume reconnect backoff where appropriate.
2 participants
Summary
Tests