Log "version" message IP addresses in client connect summary #4608

pull jgarzik wants to merge 1 commits into bitcoin:master from jgarzik:2014_log_remote changing 1 files +7 −1

jgarzik commented at 2:59 AM on July 31, 2014: contributor
The only other method of logging remote addresses is via
```
     -logips=1 -debug=net
```
which increases the logged activity by 100x or more.
jgarzik added the label Improvement on Jul 31, 2014
jgarzik added the label Docs and Output on Jul 31, 2014
laanwj commented at 6:16 AM on July 31, 2014: member

ACK

Log "version" message IP addresses in client connect summary

The only other method of logging remote addresses is via

     -logips=1 -debug=net

which increases the logged activity by 100x or more.

f7850dcca1

in src/main.cpp:None in c8722eaa6f outdated

3632 | +        string remoteAddr;
3633 | +        if (fLogIPs)
3634 | +            remoteAddr = " from " + pfrom->addr.ToStringIP();
3635 | +
3636 | +        LogPrintf("receive version message%s: %s: version %d, blocks=%d, us=%s, peer=%d\n",
3637 | +                  remoteAddr.c_str(), pfrom->cleanSubVer, pfrom->nVersion,

laanwj commented at 7:04 AM on July 31, 2014:

Please remove the c_str()

jgarzik commented at 3:45 PM on July 31, 2014: contributor

Updated to remove .c_str() from LogPrintf() call.
BitcoinPullTester commented at 4:01 PM on July 31, 2014: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4608_f7850dcca1bd04c7669b6895df4d23427f5b8c09/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

in src/main.cpp:None in f7850dcca1

3627 | @@ -3628,7 +3628,13 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv,
3628 |  
3629 |          pfrom->fSuccessfullyConnected = true;
3630 |  
3631 | -        LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, peer=%d\n", pfrom->cleanSubVer, pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString(), pfrom->id);
3632 | +        string remoteAddr;
3633 | +        if (fLogIPs)
3634 | +            remoteAddr = " from " + pfrom->addr.ToStringIP();
3635 | +
3636 | +        LogPrintf("receive version message%s: %s: version %d, blocks=%d, us=%s, peer=%d\n",

Diapolo commented at 6:56 AM on August 1, 2014:

This looks weird message%s:? Ahhh you are constructing that string above or it's empty... sorry :).

laanwj commented at 7:05 AM on August 1, 2014:

right, the space is in the string itself because it's optional

laanwj commented at 1:35 PM on August 3, 2014: member
The ::: looks a bit strange if the version message comes from localhost:
```
receive version message from ::: /Satoshi:0.9.2.1/: version 70002, blocks=271596, us=88.190.14.21:58387, peer=1
```
I'd prefer to make the peer address part of the message, for example:
```
receive version message /Satoshi:0.9.2.1/: version 70002, blocks=271596, us=88.190.14.21:58387, peer=1, peeraddr=::
```
Edit: also, if we use ToString instead of .addr.ToString() it will show the port as well, though I'm not sure how useful that is, but before 2e36866fecb7420cd73047a7aa762a6e5e225695 we had that
laanwj commented at 2:00 PM on August 3, 2014: member
Proposed implementation: https://github.com/laanwj/bitcoin/tree/2014_07_peeraddr
```
receive version message: /Satoshi:0.9.1/: version 70002, blocks=271598, us=94.242.252.41:38372, peer=1, peeraddr=[::]:0
```
jgarzik commented at 2:20 PM on August 3, 2014: contributor

I saw no value in printing the remote originating TCP port, and used ToStringIP().

Outside of that nit, your version is fine with me.
jgarzik referenced this in commit 70b9d36a2c on Aug 4, 2014
laanwj commented at 3:01 PM on August 4, 2014: member

Merged via 70b9d36
laanwj closed this on Aug 4, 2014
reddink referenced this in commit 288d2e319e on May 27, 2020
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels