Matthew Zipkin

@mzumsande This summary looks correct to me. Point (1) is covered by the functional test. You are also right that by default most nodes will not need the flag since...

Thanks so much for taking a look Gleb and for your insight, yeah I have the same concerns. That attack vector is not super trivial, the attacker needs to both...

I guess allowing more connections beyond the max is a DoS vector, which could potentially crash a node and break all existing connections anyway... hm.

@naumenkogs I added one more commit that limits the number of simultaneous forced-inbound connections to 8. I didn't add it as an init option yet but we could if you...

@willcl-ark thanks for reviewing, addressed your feedback except for the new test but always open to writing more tests so lemme know if you think something is uncovered there.

Rebasing on master hopefully to fix Windows CI failure

Rebased on master again, thanks @willcl-ark for your review. Hoping to get some more feedback from @mzumsande and @naumenkogs to proceed, or abandon

@naumenkogs If the user has *also* set a low `maxconnections` value then [`SelectNodeToEvict()`](https://github.com/bitcoin/bitcoin/blob/19d1ba1b4141f744cca291ff2d99d0b8ffcf946d/src/node/eviction.cpp#L186-L197) might return null. Quick math 4+8+4+8+4 = 28 protected nodes. Plus 8 full outbound and 2 block...

Thanks for the review Gleb, I also rebased on master to fix conflicts

This also closes https://github.com/bitcoin/bitcoin/issues/21670 ;-)