Addnode peers connected to us via the cjdns network are currently not detected by CConnman::GetAddedNodeInfo(), i.e. fConnected is always false. This causes the following issues:
RPC getaddednodeinfo incorrectly shows them as not connected
CConnman::ThreadOpenAddedConnections() continually retries to connect them
Fix the issue and add a unit regression test. Extracted from #28248. Suggest running the test with:
./src/test/test_bitcoin -t net_peer_connection_tests -l test_suite
Addnode (manual) peers connected to us via the cjdns network are currently not
detected by CConnman::GetAddedNodeInfo(), i.e. fConnected is always false.
This causes the following issues:
- RPC `getaddednodeinfo` incorrectly shows them as not connected
- CConnman::ThreadOpenAddedConnections() continually retries to connect them
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
This straightforward fix (5f53e6c073d089598a444436d74e1f5c1615030b) was previously ACKed by @vasild (https://github.com/bitcoin/bitcoin/pull/28248#pullrequestreview-1695032063 and Concept ACKed by @brunoerg (https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-1777344970) with a request to add test coverage, which I’ve added here.
(Thank you @luke-jr for updating bitcoin knots.) @vasild @brunoerg mind having a look (thanks!)
ACK d0b047494c28381942c09d0cca45baa323bfcffc
Built and tested on arm/macOS. It’s a simple fix to recognize CJDNS addresses in GetAddedNodeInfo(). Otherwise CService::IsValid() will fail because the CJDNS prefix is in a reserved range. Confirmed the test fails on master and passes with the branch.
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA256
2
3ACK d0b047494c28381942c09d0cca45baa323bfcffc
4-----BEGIN PGP SIGNATURE-----
5
6iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmZE+NUACgkQ5+KYS2KJ
7yTqRLA/+PTzgnOENcfPRDjRTqOrwzxdHPDW8xHu+m0VcLZO0Uv4HwIW8ArZk7DJB
8va3z0Y4gIWYvpLwLR+pUlSFqsLKmgsi+gYSVkGK3sGBAGvTH8ibNfKWKdSxzQaAs
9PHkutSbYFchel2aO7g/cVzNsD4IfbWvGMmxj2f4kNX0RcWG49IHi3xlMh7ppqn8x
10NjS+Xl8kzCgz8oYPQVh7KMKOLEeu+FcoaKRrWhgkhWn6A+CNRbyxwcZYXWrx1oY5
11uK+tgnnbvRoYaTg7BTluFfitOqUGPnsMWw52a+vPbtq0lMbtLGby6nPrDwlr1jCn
12HPMxz4klxHoqYXJa138Exywyck0Ziu4eqeq/jzi1vQO4KBm6MaiHWrBpfjr/i5uS
13Vc/eWPzlTHXktT4MDOb2Mtgfrcu35zQlaU4HMCkLnJGhtFDRhiab99kGHbq+h/LF
14kVrePOsVStPNg0s5UwxQIYdEBw4UXePCCPRwe9xxi7UoGGq8mmX6W7AFFn90bepQ
15n6a579L6y9olxEr0+Xh863B0esen6wh7znCIm8YJfBHoNJZmOe3tTiWNn0+HQF9e
16lqat+HDHY6H24jWSSlqpIvYtHMMe/FeP/5rNldWIdLxIAl3Nc3RPnfmYOTnnZv7F
17djr3j+Tk1sIl7hKNqYg9eFcoIzRyLqecJOPDxVrK3eCPVdM12OA=
18=u4ll
19-----END PGP SIGNATURE-----
pinheadmz’s public key is on keybase
Confirmed this branch fixes the issue in production on mainnet as well:
0$ bccli getaddednodeinfo
1[
2 {
3 "addednode": "fc32:17ea:e415:c3bf:9808:149d:b5a2:c9aa",
4 "connected": false,
5 "addresses": [
6 ]
7 },
8 {
9 "addednode": "fcc7:be49:ccd1:dc91:3125:f0da:457d:8ce",
10 "connected": true,
11 "addresses": [
12 {
13 "address": "[fcc7:be49:ccd1:dc91:3125:f0da:457d:8ce]:8333",
14 "connected": "outbound"
15 }
16 ]
17 }
18]
