Skip to content

Conversation

@pstratem
Copy link
Contributor

@pstratem pstratem commented May 16, 2018

Setting the send flag to false can be replaced by simply returning.

@maflcko maflcko added the P2P label May 16, 2018
@pstratem
Copy link
Contributor Author

pstratem commented May 16, 2018

the send flag effectively exits the function whenever it's set to false, this simply actually returns

@sipa
Copy link
Member

sipa commented May 16, 2018

Yes, I can read the code. But can you actually put that in the PR description? The description goes into the merge commit for example, and is helpful for people investigating the history of code.

@pstratem pstratem force-pushed the 2018-05-16-processgetblockdata branch from 1720603 to 311af6c Compare May 16, 2018 23:22
@pstratem pstratem changed the title Simplify ProcessGetBlockData execution Simplify ProcessGetBlockData execution by removing send flag May 16, 2018
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should return if pindex is not defined

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed good catch

@pstratem pstratem force-pushed the 2018-05-16-processgetblockdata branch from 311af6c to 660e484 Compare May 17, 2018 01:00
Setting the send flag to false can be replaced by simply returning.
@pstratem pstratem force-pushed the 2018-05-16-processgetblockdata branch 2 times, most recently from 31214e8 to b4b6cb4 Compare May 17, 2018 01:24
@fanquake
Copy link
Member

Travis:

11.84s$ if [ "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then contrib/devtools/lint-all.sh; fi
./test/functional/p2p_invalid_getdata.py:31:1: W293 blank line contains whitespace
^---- failure generated from contrib/devtools/lint-python.sh

@pstratem pstratem force-pushed the 2018-05-16-processgetblockdata branch from b4b6cb4 to 261026b Compare May 17, 2018 02:08
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2018

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're using 2 nodes below, index 0 and 1, is that right?

@pstratem pstratem force-pushed the 2018-05-16-processgetblockdata branch 3 times, most recently from a1b4bb4 to dd5ee7f Compare May 19, 2018 23:49
@DrahtBot
Copy link
Contributor

Needs rebase

# Setup the p2p connections and start up the network thread.
self.test_node = self.nodes[0].add_p2p_connection(TestP2PConn())

network_thread_start()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: The thread is always running on master. Remove this line.

@fanquake
Copy link
Member

I've rebased and fixed the nit in #13670.

@fanquake fanquake closed this Jul 15, 2018
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants