Repository navigation
tox_bootstrap should return an error on unrecoverable getnodes failure #647
Description
I'm trying to use tox in a local-network-only setup. I've setup iptables to block all non-local traffic from a user running tox. My client still reports that it bootstraps, which is alarming, but I think false.
When tox_bootstrap is called, DHT_bootstrap calls getnodes which returns either -1 or usually 113 for me, from sendpacket returning "no route to host", which is good.
But DHT_bootstrap discards this return value and returns void, then tox_bootstrap returns pass/fail just based on net_getipport, not based on whether DHT_bootstrap actually succeeded (or even sent). This results in tox_boostrap returning true for me, as count is always 1, and then the client (qTox in my case) reporting that I'm successfully connected to the DHT.
I suppose it's not really possible that tox_bootstrap return whether the bootstrapping succeeded since it would need to wait for a response (I assume, but that would be ideal) since that would block too long, but at least returning whether the bootstrap request was sent would be good.
Activity
nurupo commentedon Dec 27, 2017
Yep, that sounds like a bug that needs fixing. If
tox_bootstrap()fails to send a packet, thentox_bootstrap()return value should indicate failure.Now, I think you are confusing something here. The
tox_bootstrap()call doesn't tell whether you have successfully bootstraped or not, all it does it just sending a packet somewhere. You don't know if you have successfully bootstrapped until toxcore's main loop runs for a few iterations and it processes the packets it has received from the bootstrap nodes replying to the packet you have sent intox_bootstrap(). If toxcore does get response from bootstrap nodes, toxcore calls the callback you have set usingtox_friend_connection_status_cbsaying that it got connected. If toxcore doesn't get any response from bootstrap nodes, it doesn't call the callback. Most clients wait for some about of time and if they weren't told by toxcore that they have went online, they will assume that bootstrapping failed and will try to re-bootstrap. They don't interpret the success return value oftox_bootstrap()as the fact that they did indeed successfully bootstrapped and are now connected to the DHT. If some client does that, then they are doing it wrong. Pretty sure qTox doesn't (at least didn't in the past) do that.anthonybilinski commentedon Dec 27, 2017
Ah you are correct. qTox logs "Successfully bootstrapped from..." but doesn't actually change state to online. I only see myself as online once I have a friend come online on the network who I connect to. Once this happens, tox_self_get_connection_status returns true, and qTox changes my status to online.
I suppose it's reasonable to have tox_self_get_connection_status return true even if only connected to a local network, since there's not really a "true" DHT, as far as the core is concerned. I'll submit a PR to propagate the sendto error soon and leave the tox_self_get_connection_status behaviour unchanged.
nurupo commentedon Dec 27, 2017
It returns true as long as it's connected to at least one other DHT node, either bootstrap or not. It is indeed a bit misleading in the setting of using Tox on a local network with just 2 peers, as when one peer goes offline, the other peer's toxcore will also go offline as it wouldn't be connected to any active node anymore. The person behind the second peer might think that there is something wrong with their network or the Tox client since it says that they are offline, but everything is actually fine, it's just that there is no one else that the Tox client is connected to.
anthonybilinski commentedon Dec 27, 2017
I see similar behaviour when calling tox_add_tcp_relay(), which creates the connection but doesn't yet send any data, so returns success. When do_TCP_connection() is called, the connection is always "TCP_CLIENT_CONNECTING", and client_send_pending_data() returns an error, but there's no easy way to propagate that to the caller since it's async. Unlike tox_bootstrap() I'm not sure this is easily fixable. "Fixing" tox_bootstrap() so that it returns an error when it fails to send but leaving tox_add_tcp_relay() to still return success diverges the behaviour of the two. Is this desirable? Maybe the return values of tox_bootstrap() and tox_add_tcp_relay() should just not be taken too seriously, as any client is going to keep trying to bootstrap until they are connected anyway.
[-]Bootstrap succeeds even on isolated network[/-][+]`tox_bootstrap` should return an error on unrecoverable `getnodes` failure[/+]toxcore: update to version 0.2.18
toxcore: update to version 0.2.18
toxcore: update to version 0.2.18