Drop peers with false ENR subnet advertising

[zilm13]

PR Description

Include attnets, sync committees, das subnets

Fixed Issue(s)

Documentation

• I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

• I thought about adding a changelog entry, and added one if I deemed necessary.

[StefanBratanov]

So I have been looking at the code and this is called here:

    peerSelectionStrategy
        .selectPeersToDisconnect(network, peerPools)
        .forEach(
            peerToDrop ->
                peerToDrop.disconnectCleanly(DisconnectReason.TOO_MANY_PEERS).finishTrace(LOG));

So it disconnects for the reason TOO_MANY_PEERS. I wonder if it is better to have a different method which checks the false advertising peers and then fails with a new DisconnectReason. Probably something like FALSE_ADVERTISEMENT or something like that. Otherwise TOO_MANY_PEERS sounds wrong.

[lucassaldanha]

Maybe we can change this method to return an object with the list of peers to disconnect and the reason for that group. This way, we can always report on the exact reason for disconnecting.

[StefanBratanov]

Maybe we can change this method to return an object with the list of peers to disconnect and the reason for that group. This way, we can always report on the exact reason for disconnecting.

I like that actually, I guess more like a map DisconnectReason,List<Peer>

[zilm13]

How about this? d92d991
I cannot introduce a reason, they are specced (actually goodbye messages are specced, not the reasons, but this doesn't change much, we could use existing I think)

[lucassaldanha]

Maybe we could add a debug entry here saying we are downscoring peer X (not sure if we log this somewhere else). This will make it a bit easier to see this mechanism in action. The same applies to the other checks below.

[zilm13]

Added daf1b10

[lucassaldanha]

Is this NOOP correct here?

[zilm13]

It's test fixtures, don't need it at this point there, I think

[zilm13]

Version before adding cache:

23160 calls for 3 hours (~128 calls/minute)

50th percentile: 9 ms
90th percentile: 810 ms
95th percentile: 1487 ms
99th percentile: 3078 ms

Calls are made on different nio event threads
highest 10-20% looks very bad, looking for ways to improve

[zilm13]

Version with cache:

23408 calls during 3 hours (130 calls per minute)

50th percentile: 9 ms
90th percentile: 921 ms
95th percentile: 1696 ms
99th percentile: 3410 ms

Cache is not helping at all, trying another approach

[zilm13]

It's a final version.
No cache. Run every 5 seconds. Runtime is even worse, but is it important? It runs on separate thread and rarely (but it's nio thread)

50th percentile: 97 ms
90th percentile: 2365 ms
95th percentile: 3068 ms
99th percentile: 4386 ms

~6000 calls during 10 hours

[zilm13]

if we decide no merge, don't forget to make a pr for renaming subnet to group where it's applicable, it should be merged anyway

[tbenr]

I tested this on fusaka-devnet-5 and seems like we consider peer lying on column subnets