Skip to content

Conversation

theuni
Copy link
Member

@theuni theuni commented Oct 28, 2014

It shouldn't know or care about bitcoind's chain param selection. MessageStartChars is changed to be passed into CMessageHeader's ctor rather than having it pulled from the current app state.

This isn't necessary for the current consensus lib work, but PR'ing now anyway since it's pretty straightforward and non-intrusive. Fine to wait until post-0.10 if preferred.

@sipa
Copy link
Member

sipa commented Oct 28, 2014

utACK

@TheBlueMatt
Copy link
Contributor

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

utACK commithash 829c4362675054c6c7e48a2706a8babf84fdf7cd
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBAgAGBQJUUAQGAAoJEIm7uGY+LmXO2lUP/1KTULDTSnsn711Nu/U6ascd
hoeLBZRtxoyGGlgEhVERdk7y/olkf2yOc40zkLJ1yRkzZz0pyYw73XOp9axesrCo
AfzEkpyoekJtT+HP6be/3WERlrin6WIVAJnaNyScj66G+dtnZlrljnpegZPw87P+
5Uy98015esEVAU4mnv0ShX5NASAb+tks6MQlz7pGYM+dNqi6ONrRHPzsIrLry5zE
TVfu0/0uUhO1DEiBnbKP7CklbMshMUkig2RbP16qP2sjDuhDpS9OXi+XWlerrklh
iXxuaym8ws8MOKaAJsv26zw5vIvipl6Q6anT2vwNUaLbWXsK2Lhu1a1NUCXPMoLh
rIzEoqai0dePfTTIfCK2M5l5QCCHs2GIV46EuQMHSl0cJgE6ANT63KX4GrW3yp3r
EgRjfsNEYe3Td+J029lf79DQTBVzOwBED0fAowBEoK3BS38cft0zw8kCR9TNXQPy
YAnEOpYrGxED23oX6u52Ppn3ERsZm8sRUMVMzNq9edCrp26crwsfFZx5u0PyXaZ3
h7/Q7V6qB6wmieV+uY0QbSqUGPf5JHh5Bzg2CriNLruPHVOEsViaPNZqg/DrK/9o
9H4tFMeCkeJWERB7/QPV8zjz18SKGoVLfNVqAuwLaUN/qVa26Qs3E6+yQLdd1yw8
SqhzEvQ14MhezMOxImwr
=RuMZ
-----END PGP SIGNATURE-----

@sipa
Copy link
Member

sipa commented Nov 4, 2014

Needs rebase.

@theuni theuni force-pushed the reducedeps8 branch 2 times, most recently from c288169 to 4718176 Compare November 4, 2014 21:12
@theuni
Copy link
Member Author

theuni commented Nov 4, 2014

rebased

@jgarzik
Copy link
Contributor

jgarzik commented Nov 4, 2014

ut ACK

@laanwj
Copy link
Member

laanwj commented Jan 16, 2015

@theuni We can easily go one step further here and explicitly pass the start characters into CNetMessage too: eb9d140 . I like it slightly better as it moves the call to chainparams from the header to the net implementation file.

@theuni
Copy link
Member Author

theuni commented Feb 4, 2015

@laanwj Yes, nice change. I've added that here.

Edit: Added as a new commit to preserve the previous ACKs. Will squash when you're ready.

@jtimon
Copy link
Contributor

jtimon commented Feb 20, 2015

ut ACK

It shouldn't know or care about bitcoind's chain param selection
@theuni
Copy link
Member Author

theuni commented Feb 26, 2015

This had conflicts, so it's been squashed and rebased.

@sipa
Copy link
Member

sipa commented Mar 1, 2015

re-utACK

1 similar comment
@jtimon
Copy link
Contributor

jtimon commented Mar 2, 2015

re-utACK

@sipa sipa merged commit eec3713 into bitcoin:master Mar 7, 2015
sipa added a commit that referenced this pull request Mar 7, 2015
eec3713 make CMessageHeader a dumb storage class (Cory Fields)
random-zebra added a commit to PIVX-Project/PIVX that referenced this pull request Sep 7, 2020
30d5c66 net: correct addrman logging (Fuzzbawls)
8a2b7fe Don't send layer2 messages to peers that haven't completed the handshake (Fuzzbawls)
dc10100 [bugfix] Making tier two thread interruptable. (furszy)
2ae76aa Move CNode::addrLocal access behind locked accessors (Fuzzbawls)
470482f Move CNode::addrName accesses behind locked accessors (Fuzzbawls)
35365e1 Move [clean|str]SubVer writes/copyStats into a lock (Fuzzbawls)
d816a86 Make nServices atomic (Matt Corallo)
8a66add Make nStartingHeight atomic (Matt Corallo)
567c9b5 Avoid copying CNodeStats to make helgrind OK with buggy std::string (Matt Corallo)
aea5211 Make nTimeConnected const in CNode (Matt Corallo)
cf46680 net: fix a few races. (Fuzzbawls)
c916fcf net: add a lock around hSocket (Cory Fields)
cc8a93c net: rearrange so that socket accesses can be grouped together (Cory Fields)
6f731dc Do not add to vNodes until fOneShot/fFeeler/fAddNode have been set (Matt Corallo)
07c8d33 Ensure cs_vNodes is held when using the return value from FindNode (Matt Corallo)
110a44b Delete some unused (and broken) functions in CConnman (Matt Corallo)
08a12e0 net: log an error rather than asserting if send version is misused (Cory Fields)
cd8b82c net: Disallow sending messages until the version handshake is complete (Fuzzbawls)
54b454b net: don't run callbacks on nodes that haven't completed the version handshake (Cory Fields)
2be6877 net: deserialize the entire version message locally (Fuzzbawls)
444f599 Dont deserialize nVersion into CNode (Fuzzbawls)
f30f10e net: remove cs_vRecvMsg (Fuzzbawls)
5812f9e net: add a flag to indicate when a node's send buffer is full (Fuzzbawls)
5ec4db2 net: Hardcode protocol sizes and use fixed-size types (Wladimir J. van der Laan)
de87ea6 net: Consistent checksum handling (Wladimir J. van der Laan)
d4bcd25 net: push only raw data into CConnman (Cory Fields)
b79e416 net: add CVectorWriter and CNetMsgMaker (Cory Fields)
63c51d3 net: No need to check individually for disconnection anymore (Cory Fields)
07d8c7b net: don't send any messages before handshake or after fdisconnect (Cory Fields)
9adfc7f net: Set feelers to disconnect at the end of the version message (Cory Fields)
f88c06c net: handle version push in InitializeNode (Cory Fields)
04d39c8 net: construct CNodeStates in place (Cory Fields)
40a6c5d net: remove now-unused ssSend and Fuzz (Cory Fields)
681c62d drop the optimistic write counter hack (Cory Fields)
9f939f3 net: switch all callers to connman for pushing messages (Cory Fields)
8f9011d connman is in charge of pushing messages (Cory Fields)
f558bb7 serialization: teach serializers variadics (Cory Fields)
01ea667 net: Use deterministic randomness for CNode's nonce, and make it const (Cory Fields)
de1ad13 net: constify a few CNode vars to indicate that they're threadsafe (Cory Fields)
34050a3 Move static global randomizer seeds into CConnman (Pieter Wuille)
1ce349f net: add a flag to indicate when a node's process queue is full (Fuzzbawls)
5581b47 net: add a new message queue for the message processor (Fuzzbawls)
701b578 net: rework the way that the messagehandler sleeps (Fuzzbawls)
7e55dbf net: Add a simple function for waking the message handler (Cory Fields)
47ea844 net: record bytes written before notifying the message processor (Cory Fields)
ffd4859 net: handle message accounting in ReceiveMsgBytes (Cory Fields)
8cee696 net: log bytes recv/sent per command (Fuzzbawls)
754400e net: set message deserialization version when it's time to deserialize (Fuzzbawls)
d2b8e0a net: make CMessageHeader a dumb storage class (Fuzzbawls)
cc24eff net: remove redundant max sendbuffer size check (Fuzzbawls)
32ab0c0 net: wait until the node is destroyed to delete its recv buffer (Cory Fields)
6e3f71b net: only disconnect if fDisconnect has been set (Cory Fields)
1b0beb6 net: make GetReceiveFloodSize public (Cory Fields)
229697a net: make vRecvMsg a list so that we can use splice() (Fuzzbawls)
d2d71ba net: fix typo causing the wrong receive buffer size (Cory Fields)
50bb09d Add test-before-evict discipline to addrman (Ethan Heilman)

Pull request description:

  This is a combination of multiple upstream PRs focused on optimizing the P2P networking flow after the introduction of CConnman encapsulation, and a few older PRs that were previously missed to support the later optimizations. The PRs are as follows:

  - bitcoin#9037 - net: Add test-before-evict discipline to addrman
  - bitcoin#5151 - make CMessageHeader a dumb storage class
  - bitcoin#6589 - log bytes recv/sent per command
  - bitcoin#8688 - Move static global randomizer seeds into CConnman
  - bitcoin#9050 - net: make a few values immutable, and use deterministic randomness for the localnonce
  - bitcoin#8708 - net: have CConnman handle message sending
  - bitcoin#9128 - net: Decouple CConnman and message serialization
  - bitcoin#8822 - net: Consistent checksum handling
  - bitcoin#9441 - Net: Massive speedup. Net locks overhaul
  - bitcoin#9609 - net: fix remaining net assertions
  - bitcoin#9626 - Clean up a few CConnman cs_vNodes/CNode things
  - bitcoin#9698 - net: fix socket close race
  - bitcoin#9708 - Clean up all known races/platform-specific UB at the time PR was opened
    - Excluded bitcoin/bitcoin@512731b and bitcoin/bitcoin@d8f2b8a, to be done in a separate PR

ACKs for top commit:
  furszy:
    code ACK 30d5c66 , testnet sync from scratch went well and tested with #1829 on top as well and all good.
  furszy:
     mainnet sync went fine, ACK 30d5c66 .
  random-zebra:
    ACK 30d5c66 and merging...

Tree-SHA512: 09689554f53115a45f810b47ff75d887fa9097ea05992a638dbb6055262aeecd82d6ce5aaa2284003399d839b6f2c36f897413da96cfa2cd3b858387c3f752c1
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 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.

6 participants