Bugfix: use global ::fRelayTxes instead of CNode in version send #8102

pull sipa wants to merge 1 commits into bitcoin:master from sipa:oopsrelay changing 1 files +1 −1

sipa commented at 4:47 PM on May 26, 2016: member

#8049 changed the calls to GetBoolArg("-blocksonly") to a global boolean fRelayTxes. Inside CNode::PushVersion, fRelayTxes refers to the CNode instance variable with the same name, however, causing the connecting node to always send false here, and never receive any transactions.

I've attempted to add a unit test for this, but we don't seem to ever test receiving relayed transactions by a connecting node only...
sipa commented at 4:59 PM on May 26, 2016: member

I'll work on a better test for this, but I propose merging quickly, as relay of transactions is currently broken in master.
theuni commented at 5:11 PM on May 26, 2016: member

yikes. utACK.

As an aside, we should really strive to avoid shadowing vars, and enable -Wshadow.

Also, as a follow-up, this should probably be renamed fBlocksonly so that it's more clear what it's for.
Use global ::fRelayTxes instead of CNode one 52b02ecd6d
sipa force-pushed on May 26, 2016
sipa merged this on May 26, 2016
sipa closed this on May 26, 2016
sipa referenced this in commit 425278d17b on May 26, 2016
MarcoFalke cross-referenced this on May 26, 2016 from issue avoid shadowing vars, and enable -Wshadow by MarcoFalke
laanwj commented at 7:32 PM on May 26, 2016: member

Good catch!
laanwj cross-referenced this on Jun 9, 2016 from issue developer notes: updates for C++11 by kazcw
laanwj cross-referenced this on Sep 22, 2016 from issue Enable -Wshadow by default by paveljanik
bitcoin locked this on Sep 8, 2021

Contributors

sipa

theuni

laanwj

Linked (view graph)

#8049 Expose information on whether transaction relay is enabled in `getnetwork`#8105 avoid shadowing vars, and enable -Wshadow #8177 developer notes: updates for C++11 #8794 Enable -Wshadow by default