New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from May 26, 2016

Conversation

Projects
None yet
3 participants
@sipa
Member

sipa commented May 26, 2016

#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

This comment has been minimized.

Show comment
Hide comment
@sipa

sipa 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.

Member

sipa commented May 26, 2016

I'll work on a better test for this, but I propose merging quickly, as relay of transactions is currently broken in master.

@theuni

This comment has been minimized.

Show comment
Hide comment
@theuni

theuni 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.

Member

theuni commented May 26, 2016

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.

@sipa sipa merged commit 52b02ec into bitcoin:master May 26, 2016

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

sipa added a commit that referenced this pull request May 26, 2016

Merge #8102: Bugfix: use global ::fRelayTxes instead of CNode in vers…
…ion send

52b02ec Use global ::fRelayTxes instead of CNode one (Pieter Wuille)

@MarcoFalke MarcoFalke referenced this pull request May 26, 2016

Closed

avoid shadowing vars, and enable -Wshadow #8105

6 of 8 tasks complete
@laanwj

This comment has been minimized.

Show comment
Hide comment
@laanwj

laanwj May 26, 2016

Member

Good catch!

Member

laanwj commented May 26, 2016

Good catch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment