Ouch, I naïvely assumed that we only bound to loopback for the tests (for obvious security reasons). Very nice find!
Very strong concept ACK
Are there any cases in which we don't want to bind to lo? Later arguments take precedence if I'm not mistaken, couldn't this just be hardcoded into args and set extra_args to have -bind=0.0.0.0 if other behaviour is required?
@esotericnonsense Binding to 0.0.0.0 means "bind to all local addresses" (which includes loopback, as I suspect you probably already know), and is often a default for many server daemons, so I don't see why it should be necessary to bind to loopback specifically (unless, of course, -bind=127.0.0.1 was specified).
On a side-note: When binding to 0.0.0.0, on some systems that support IPv6 this has the effect of also binding to ::1 in addition to 127.0.0.1 (I encountered this behaviour on Windows 7 a few years ago), but this really shouldn't be of any concern as it's a stack-implementation matter that I generally regard as being beyond the scope of most server daemons and other socket-listening applications.
@esotericnonsense note that Cannot set -bind or -whitebind together with -listen=0.
A little more correct would be to call it "bind only to localhost" instead of "bind to localhost", because the default will bind (P2P) on all interfaces including localhost, but concept ACK.
Rebased
213 | @@ -213,7 +214,7 @@ def run_test(self): 214 | 215 | # Public helper methods. These can be accessed by the subclass test scripts. 216 | 217 | - def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, binary=None): 218 | + def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, binary=None, bind=True):
Nit, but should this argument also be bind_to_localhost_only?
bind is a bit ambiguous.
How is this argument passed along in this function?
it isn't used
It doesn't make much sense to me to add the extra bind_to_localhost_only argument to the initialize_datadir() function. That function doesn't take any other arguments related to bitcoind config.
It seems more architecturally sound to me to have the add_nodes() function update the config file directly, perhaps through an update_config_file() method on the TestNode class.
Nit addressed; I renamed bind to bind_to_localhost_only. I also removed it from add_nodes, not sure why I put it there in the first place.
Prevents OSX firewall allow-this-application-to-accept-inbound-connections
permission popups and is generally safer.
To prevent binding to 127.0.0.1, set self.bind_to_localhost_only = False.
utACK b156ff7c30175f5d23bde97a56a2c34af2196466. Will test tomorrow.