I've tested this on two mainnet nodes for a while now.
Seems no problem in keeping connections to existing nodes, and I have not noticed any non-NODE_CASH nodes in my outbounds anymore.

The relevant netcode is unfortunately not easily unit-testable or system-testable except for manual procedure as indicated in Test Plan.

I like the addition of the command-line argument. I can imagine use cases where someone might want this turned off explicitly (say, when synching up or whatnot). We should leave the command-line argument in.

I was reading over the code and I saw this in net.cpp line 1876:

// only connect to full nodes
if ((addr.nServices & REQUIRED_SERVICES) != REQUIRED_SERVICES) {
    continue;
}

So I thought that would mean -prefcashpeer=0 wouldn't work. But it does work. *scratches head*. Maybe I am misreading the code... Can you enlighten me on what's going on?

Anyway this works as intended...

UPDATE: Yeah the REQUIRED_SERVICES constant means even with -prefcashpeer=0, we won't relay or store addresses that aren't cash. Is this intended? (see net_processing.cpp line 1708).

Is this what we want for -prefcashpeer=0?

No, you're right, -noprefcashpeer is not working as intended. I'd need to elaborate the condition you pointed at.
If we want to keep the option, that is.

In D457#7844, @freetrader wrote:

No, you're right, -noprefcashpeer is not working as intended. I'd need to elaborate the condition you pointed at.
If we want to keep the option, that is.

I like the option a lot, but I know deadalnix is against it. if you do keep the option the easiest thing to do really is take the info for required services from the config that gets passed down to net/connman/etc and get rid of the REQUIRED_SERVICES as a static global...

Not hard to fix, really.

I like the option, FWIW.

Prediction: deadalnix will shoot it down though.. and ask you to eliminate the option. In which case your work is much easier! :)