Use cached [compact] blocks to respond to getdata messages #9665

pull TheBlueMatt wants to merge 3 commits into bitcoin:master from TheBlueMatt:2017-02-processgetdata-cache changing 1 files +36 −18

TheBlueMatt commented at 9:19 pm on February 1, 2017: member
fanquake added the label P2P on Feb 2, 2017
TheBlueMatt commented at 7:56 pm on February 2, 2017: member

Pushed a second commit which better handles compact block caching when witnesses are not present in the given block.
TheBlueMatt force-pushed on Feb 2, 2017

in src/net_processing.cpp: in 9059eccf85 outdated

774@@ -775,6 +775,7 @@ static CCriticalSection cs_most_recent_block;
775 static std::shared_ptr<const CBlock> most_recent_block;
776 static std::shared_ptr<const CBlockHeaderAndShortTxIDs> most_recent_compact_block;
777 static uint256 most_recent_block_hash;
778+static bool fWitnessesPresentInMostRecentCompactBlock;

instagibbs commented at 4:39 pm on February 7, 2017:

nit: A comment above saying that all these fields are protected by cs_most_recent_block would be nice since it’s a growing list.

TheBlueMatt commented at 4:49 pm on February 7, 2017:

Done.

instagibbs approved
instagibbs commented at 4:44 pm on February 7, 2017: member

utACK

nit: 2nd commit could stand an explanation in message

in src/net_processing.cpp: in 4a2e1940bb outdated

1028@@ -1027,13 +1029,19 @@ void static ProcessGetData(CNode* pfrom, const Consensus::Params& consensusParam
1029                 if (send && (mi->second->nStatus & BLOCK_HAVE_DATA))
1030                 {
1031                     // Send block from disk
1032-                    CBlock block;
1033-                    if (!ReadBlockFromDisk(block, (*mi).second, consensusParams))
1034-                        assert(!"cannot load block from disk");
1035+                    std::shared_ptr<const CBlock> pblock;

sdaftuar commented at 7:56 pm on February 23, 2017:

I think the comment at line 1031 above should be moved down into the else, or deleted.

TheBlueMatt commented at 8:41 pm on February 23, 2017:

Done.

in src/net_processing.cpp: in 4a2e1940bb outdated

1085+                                CBlockHeaderAndShortTxIDs cmpctblock(*pblock, fPeerWantsWitness);
1086+                                connman.PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, cmpctblock));
1087+                            }
1088                         } else
1089-                            connman.PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, block));
1090+                            connman.PushMessage(pfrom, msgMaker.Make(nSendFlags, NetMsgType::BLOCK, *pblock));

sdaftuar commented at 8:31 pm on February 23, 2017:

style nit: i think this is supposed to have curly braces around it now?

TheBlueMatt commented at 8:42 pm on February 23, 2017:

Done.

sdaftuar commented at 8:32 pm on February 23, 2017: member

Code review ACK apart from a couple nits. Will test.
Use cached [compact] blocks to respond to getdata messages efc135ff6d
Cache witness-enabled state with recent-compact-block-cache c47f5b7982
Add comment about cs_most_recent_block coverage b49ad44efe
TheBlueMatt force-pushed on Feb 23, 2017
sdaftuar commented at 8:56 pm on February 23, 2017: member

ACK b49ad44
sipa commented at 4:11 am on February 24, 2017: member

utACK b49ad44efeaff66806d2c142273f70e2a4ddfb9d
laanwj merged this on Apr 13, 2017
laanwj closed this on Apr 13, 2017
laanwj referenced this in commit eab00d96df on Apr 13, 2017
codablock referenced this in commit 3634edd151 on Mar 12, 2019
codablock referenced this in commit d1db98c67a on Mar 12, 2019
MarcoFalke locked this on Sep 8, 2021

Contributors
TheBlueMatt instagibbs sdaftuar sipa

Labels
P2P