Multiple connections to same i2p address

ghost commented at 10:46 pm on November 19, 2022: none

This behavior is okay for ipv4 based on https://bitcoin.stackexchange.com/questions/87739/can-a-bitcoin-node-create-an-outgoing-connection-to-a-inbound-node

However, making multiple connections to same i2p peer makes no sense IMO

Expected behavior

No connections (outbound/inbound) should be initiated with i2p address that already exists as one of the peer.

Actual behavior

Screenshot 2022-11-20 035027

To reproduce

Setup 3 nodes such that node2 and node3 works with i2p
Connect node2 manually with addnode in config to node1
Connect node3 manually with node2 using i2p address
Observe result for getpeerinfo in a few minutes or check peers list in bitcoin-qt

System information

Bitcoin Core Master Branch, v24.0rc2 and v24.0rc4

unknown added the label Bug on Nov 19, 2022

vasild commented at 1:43 pm on November 23, 2022: contributor

There is already a check to avoid connecting to an already connected address. See:

#22559 (comment) and https://bitcoin.stackexchange.com/a/116097/129003

What you are observing is probably due to the race in that check described above. For the second screenshot I guess what happened is that while you initiated the manual connection, after the connection attempt started, but before it completed an automatic outbound was initiated to the same address and later that address connected to us for the inbound :disappointed:. This is not I2P specific, but more likely to be seen with I2P because establishing a connection takes longer.

What I think can be improved:

After accepting an incoming connection, drop it if we already have a connection to the same address (ignore the port).
Fix the race by maintaining a list of “attempting to connect to” addresses and do not initiate more connections to them.
Or maybe, periodically, e.g. every 5 minutes, check for duplicates in CConnman::m_nodes and drop the redundant ones, maybe ignoring the port.

vasild commented at 10:09 am on November 24, 2022: contributor

Preventing incoming connections from an address we already have a connection to could block legit cases where multiple distinct nodes run behind a common address. At first I thought that this is ok, since the disconnected node will find another random peer to connect to. But then, having multiple connections with the same address, even if it is the same node, is a minor issue too. Maybe not worth the effort to change that. Or do only for I2P.

ghost commented at 8:21 pm on November 24, 2022: none

For the second screenshot I guess what happened is that while you initiated the manual connection, after the connection attempt started, but before it completed an automatic outbound was initiated to the same address and later that address connected to us for the inbound 😞

I will try to reproduce this with deubg=net although I agree something similar happened as you described.

ghost commented at 8:21 pm on November 24, 2022: none

Fix the race by maintaining a list of “attempting to connect to” addresses and do not initiate more connections to them.

This makes sense.

ghost commented at 7:49 pm on November 26, 2022: none

For the second screenshot I guess what happened is that while you initiated the manual connection, after the connection attempt started, but before it completed an automatic outbound was initiated to the same address and later that address connected to us for the inbound 😞

I will try to reproduce this with deubg=net although I agree something similar happened as you described.

Issue is intermittent and I couldn’t collect any useful logs.

unknown closed this on Nov 26, 2022

Multiple connections to same i2p address #26537