This PR extends network permission to outbound peers (they currently only apply to inbound) provided they are connected manually. The approach is to set them via whitelist and match it only if the connection was created manually.
The rationale for the change spawns from multiple comments throughout the years, including:
The PR originally didn’t restrict the change to connections made manually, which made it a bit weird given outbound connections are mainly picked at random by our node, however the approach was changed to do so, which I think makes way more sense (giving special permissions to nodes picked at random feels scary).
The code changes are fairly small, basically patching the parser so it can accept in/out parameters. There’re some pieces of code being moved around which I haven’t found a proper motivation for (I’m asking about it in the review), plus some things that get split which do not make sense out of the box to me (also asking).
Most of the modifications relates to tests (updating the extra_args for whitelist with a new object attribute to make it easier to setup/less verbose).
I’ve tested this and tried to come up with weird ways of making it fail, such ass adding the same direction multiple times (in,in,[email protected] ), adding the direction to the same address multiple times (in,noban,in,[email protected] ) or just adding the direction ([email protected] ). For the latter I’m not sure what the expected behavior is, so also asking.
Overall it looks good and pretty harmless now that it has been narrowed down to only manual connections.
p2p: Allow whitelisting manual connections by brunoerg · Pull Request #27114 · bitcoin/bitcoin
@stratospher reached out regarding the usefulness of the patch wrt the additional code complexity. Looks like even @brunoerg may want to roll this back before it makes it into a release because he doesn’t see it being useful enough now.
Lets analyze the changes in the (non-test) parts of the codebase: