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: contributor

No description provided.
fanquake added the label P2P on Feb 2, 2017
TheBlueMatt commented at 7:56 PM on February 2, 2017: contributor

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:None 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:None 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:None 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
jnewbery cross-referenced this on Jul 31, 2017 from issue TODO for release notes 0.15.0 by MarcoFalke
codablock referenced this in commit 3634edd151 on Mar 12, 2019
codablock cross-referenced this on Mar 12, 2019 from issue Backport PRs from Bitcoin required to make ActivateBestChain cs_main free on entry by codablock
codablock referenced this in commit d1db98c67a on Mar 12, 2019
bitcoin locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#9889 TODO for release notes 0.15.0