Avoid "Unknown command" messages when receiving getaddr on outbound c… #6344

pull rebroad wants to merge 1 commits into bitcoin:master from rebroad:GetAddrUnknownCommand changing 1 files +9 −6

rebroad commented at 7:55 PM on June 26, 2015: contributor

…onnections.

This bug was introduced in #5442
Avoid "Unknown command" messages when receiving getaddr on outbound connections. c48f33d080
jonasschnelli commented at 8:12 PM on June 26, 2015: contributor

Rhm... i think this PR does not change anything.

Your PR would allow the program flow to enter the else if (strCommand == "getaddr") even if if (!pfrom->fInbound),... but it would return true anyways on line 4743(https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L4743). Or do i miss something?
rebroad commented at 9:54 PM on June 26, 2015: contributor

Yes, it will exit before displaying the "unknown command" message now. On Jun 26, 2015 11:13 PM, "Jonas Schnelli" notifications@github.com wrote:

Rhm... i think this PR does not change anything.

Your PR would allow the program flow to enter the else if (strCommand == "getaddr") even if if (!pfrom->fInbound),... but it would return true anyways on line 4743( https://github.com/bitcoin/bitcoin/blob/master/src/main.cpp#L4743). Or do i miss something?

— Reply to this email directly or view it on GitHub #6344 (comment).
jonasschnelli commented at 7:05 AM on June 27, 2015: contributor

Ah. Right. Wasn't aware of the else on L4736.

utACK.
jgarzik commented at 9:38 PM on June 27, 2015: contributor

If we are changing this code, please also log this exceptional event.

concept ACK
laanwj added the label P2P on Jul 2, 2015
laanwj commented at 6:02 PM on July 2, 2015: member

Meh.

a) the message will only appear when debug=net is set b) at least with the old code it shows that something weird is happening, which is the point of verbose net logging (even though the message could be better) - after this patch it's ignored without any logging
sipa commented at 2:54 PM on July 9, 2015: member

Don't care.
laanwj commented at 3:15 PM on July 10, 2015: member

I don't think this is enough of an improvement to merge, so going to close this.
laanwj closed this on Jul 10, 2015
rebroad commented at 4:53 PM on March 1, 2016: contributor

@laanwj I'd consider this a bugfix rather than an improvement, since "getaddr" clearly is NOT an unknown command.
MarcoFalke commented at 7:11 PM on March 1, 2016: member

@rebroad The previous comments say that you are removing a line (interesting to at least some users) from the debug.log. Why not keep logging this with a proper message (as @jgarzik suggests)?
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#5442 Ignore getaddr messages on Outbound connections.#7642 Avoid "Unknown command" messages when receiving getaddr on outbound c…