Simplify ProcessGetBlockData execution by removing send flag#13670
Simplify ProcessGetBlockData execution by removing send flag#13670fanquake wants to merge 1 commit intobitcoin:masterfrom
Conversation
|
Concept ACK |
promag
left a comment
promag
left a comment
There was a problem hiding this comment.
Concept ACK, simplification is correct. Not sure about the new test.
There was a problem hiding this comment.
Specify:
from test_framework.mininode import (CInv, P2PInterface, msg_getdata)
|
utACK 4677dc25d4fa3183e745d2b2f995809845148cc8. Didn't review the test. |
fanquake
force-pushed
the
pstratem_rebased_processgetblockdata
branch
from
4677dc2 to
093ea05
Compare
|
I think it is somewhat subjective whether an early return is preferable to a status variable. ACK on the test in any case. |
There was a problem hiding this comment.
Doesn't seem like this subclass is necessary. Any reason you can't just use P2PInterface on line 23 below?
There was a problem hiding this comment.
Agree. (Sorry, my previous comment didn't apply, thus deleted)
fanquake
force-pushed
the
pstratem_rebased_processgetblockdata
branch
from
093ea05 to
077df8e
Compare
| No more conflicts as of last run. |
There was a problem hiding this comment.
What is this testing? I don't see any assertions.
There was a problem hiding this comment.
I think an earlier version of the code would crash when receiving such an inv. But yeah, somewhat agree that a new test file with all the overhead is a bit too much for testing so little
Setting the send flag to false can be replaced by simply returning.
fanquake
force-pushed
the
pstratem_rebased_processgetblockdata
branch
from
077df8e to
0cf546f
Compare
|
I've just dropped the test entirely here. |
|
utACK 0cf546f ...
... as long as these horrible single line if statements with multiple
nested parens are cleaned up soon after
|
…ata, remove send bool fa81773 style-only: Remove whitespace (MarcoFalke) fae77b9 net: Simplify ProcessGetBlockData execution by removing send flag. (Patrick Strateman) fae7c04 log: Clarify that block request below NODE_NETWORK_LIMITED_MIN_BLOCKS disconnects (MarcoFalke) Pull request description: * Clarify that "ignoring" really means "disconnect" in the log * Revive a refactor I took from #13670 ACKs for top commit: jnewbery: utACK fa81773 sipa: utACK fa81773 Tree-SHA512: 0a4fcb979cb82c4e26012881eeaf903c38dfbb85d461476c01e35294760744746a79c48ffad827fe31c1b830f40c6e4240529c71e375146e4d0313c3b7d784ca
11 participants
Rebased/nits fixed version of #13250.