Isn't there some behaviour we want when the actual value (based on number of connections, not maximum number) <= 0, like cancelling the attempt?

Isn't there some behaviour we want when the actual value (based on number of connections, not maximum number) <= 0, like cancelling the attempt?

I've checked the code below it and I think there is no danger from this value reaching <0. The only thing that actually uses nMaxInBound is:

    if (nInbound >= nMaxInbound)
    {
        if (!AttemptToEvictConnection()) {
            // No connection to evict, disconnect the new connection
            LogPrint("net", "failed to find an eviction candidate - connection dropped (full)\n");
            CloseSocket(hSocket);
            return;
        }
    }

So it will try to evict all connections, just like when nMaxInbound would be 0, which I think makes sense. No need for special-casing.

utACK fa1c3c2

Theoretically we could stop listening for inbound connections completely if this is <=0, however, whitelisted connections are probably an exception? There are some potential traps there that can create additional unexpected behavior changes. I think this pull is safe and we can do that later.

Theoretically we could stop listening for inbound connections completely if this is <=0, however, whitelisted connections are probably an exception?

Probably the logic should be changed further in the future, if you set maxconnections to 5 and -listen=1 -connect=gateway.host I would expect you to end up with 5 connections, not 1.

But ACK getting rid of the assert, good move for now and telling people to set maxconnections to a small number used to be common advice for people concerned with memory and bandwidth (we now use less memory per peer, so less reason)... I also checked that the negative is fine with the current code.