Prevent dual-stack spamming #275
Conversation
src/service/ip_vote.rs
Outdated
| SocketAddr::V6(socket) => *ip6_count.entry(*socket).or_insert_with(|| 0) += 1, | ||
| } | ||
| } | ||
| self.ipv4_votes.retain(|_, v| v.1 > instant); |
There was a problem hiding this comment.
does the order import for majority calculation?
I.e why do we have a test function for test_three_way_vote_draw.
We can save 2 iterations if we combine majority calculation and stale votes removal did at
ac644c8 but test_three_way_vote_draw fails
There was a problem hiding this comment.
Yeah I think your changes are better. Can't remember about that test. I think we wanted to test that if there is a draw it at least returns Some and not None. It probably should be random from the tie.
Can you make a PR to adjust the logic to what you have here.
Why don't we just mutate the hashmap in the function rather than returning a new hashmap and re-setting it?
There was a problem hiding this comment.
yeah makes sense, updated and added the function to your PR!
src/service/ip_vote.rs
Outdated
| SocketAddr::V6(socket) => *ip6_count.entry(*socket).or_insert_with(|| 0) += 1, | ||
| } | ||
| } | ||
| self.ipv4_votes.retain(|_, v| v.1 > instant); |
There was a problem hiding this comment.
yeah makes sense, updated and added the function to your PR!
Description
I've been testing Ipv6 and dual stack support.
There was a bug, where in dual-stack mode, if a router has not SNAT'd correctly they use random source ports. The
majority()voting system will never reach a majority and we will continue to ping all peers we see.We waste a lot of traffic as we are never going to reach a majority in this scenario. The correct logic is not to check for a majority on a specific port before asking more more PING's rather, if we have enough PONG responses to reach a majority if our NAT was correctly set up, then no longer send more PINGs.
This PR updates to this new logic.