Relay compact block messages prior to full block connection #9375

TheBlueMatt · 2016-12-19T07:35:33Z

First clean up net_processing to never have non-const access to CBlockIndex*es.

Then take advantage of #9026 to relay compact block announcements prior to full validation of the block. This has the (incredibly) nice property of not re-deserializing the same block over and over again in SendMessages when we go to announce the block.

Finally caches the block which was just announced to respond to getblocktxn requests without a similar deserialize (this could probably be extended to be used by other requests as well, but other requests are still going to want to lock cs_main and check that the block didn't get rejected as invalid first).

gmaxwell · 2016-12-19T07:40:19Z

could we have some benchmarks do demonstrate the gains from this? though I don't doubt them, this is a fair amount of code... and benchmarks would be important for regression testing this important behavior.

TheBlueMatt · 2016-12-19T07:58:11Z

Which part, specifically, would you like to see benchmarks on?

…

On December 18, 2016 11:40:22 PM PST, Gregory Maxwell ***@***.***> wrote: could we have some benchmarks do demonstrate the gains from this? though I don't doubt them, this is a fair amount of code... and benchmarks would be important for regression testing this important behavior. -- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: #9375 (comment)

gmaxwell · 2016-12-19T08:41:45Z

latency to relay a block across many connections would be the obvious thing?

morcos · 2016-12-21T16:50:46Z

Is there ever a case where we would accept a block ourselves but we wouldn't want to immediately announce it? I looked through all the places we call AcceptBlock/ProcessNewBlock and couldn't immediately see a problem, but want to make sure we think about it carefully. I noticed this because I found it odd you had to change the preciousblock.py test. BTW, a better change there may just be to allow duplicate-inconclusive in the list of results..

rebroad · 2016-12-21T17:26:27Z

src/net_processing.cpp

+                for (size_t i = 0; i < req.indexes.size(); i++) {
+                    if (req.indexes[i] >= block.vtx.size()) {
+                        Misbehaving(pfrom->GetId(), 100);
+                        LogPrintf("Peer %d sent us a getblocktxn with out-of-bounds tx indices", pfrom->id);


this chunk of code is duplicated elsewhere?

morcos · 2016-12-21T19:10:57Z

src/net_processing.cpp

+        // but we don't think they have this one, go ahead and announce it
+        if (state.fPreferHeaderAndIDs && (!fWitnessEnabled || state.fWantsCmpctWitness) &&
+                !PeerHasHeader(&state, pindex) && PeerHasHeader(&state, pindex->pprev)) {
+            connman->PushMessage(pnode, msgMaker.Make(NetMsgType::CMPCTBLOCK, cmpctblock));


maybe add LogPrint("net", "%s sending header-and-ids %s to peer %d..

morcos · 2016-12-21T19:35:35Z

src/net_processing.cpp

+    CNetMsgMaker msgMaker(PROTOCOL_VERSION);
+
+    LOCK(cs_main);
+    bool fWitnessEnabled = IsWitnessEnabled(pindex, Params().GetConsensus());


shouldn't this be pindex->pprev? I don't suppose it matters much, but I think you'll be thinking segwit activated 1 block early

@morcos don't you mean one block late?

morcos · 2016-12-21T19:54:13Z

This seems like its going to be super awesome.
code review ACK modulo nits, doing some testing...

TheBlueMatt · 2016-12-22T08:08:23Z

Fixed comments, note that the SendBlockTransactions function seems a bit odd now, but the goal longer-term is to move messages into their own processing groups, where such functions will be really sane.

gmaxwell · 2016-12-22T12:36:48Z

Perhaps Newpowvalidblock should be gated on there actually being any peers that request a compact block from us? Otherwise we'll go through the work of constructing a compact block there only to send it to no one. :)

Thoughts on caching the compact block like you cache the block? You would want it for non-pref peers that request it after you announce the header... and it's reasonably small.

morcos · 2016-12-22T15:25:24Z

utACK 5bc61fa

TheBlueMatt · 2016-12-22T23:31:41Z

@gmaxwell, well those requests are somewhat contradictory, so I picked the cache-compact-representation option :p.

gmaxwell · 2016-12-23T18:08:47Z

@TheBlueMatt not so! as it could skip processing when there are no peers requesting, and update caches any time it processes!

I don't think it matters except in one case:

Isolated mining node with no HB requesting peers, constructing the short block earlier will delay block validation which delays updating the create new block. I'm guessing the delay is <10ms, so probably inconsequential. But thought I'd bring it to your attention.

TheBlueMatt · 2016-12-23T18:18:36Z

Yea, I'm not worried about the processing time for calling that once. Long term I'd like to run most of the CValidationInterface callbacks into background threads, but that's probably an 0.15-targeted thing.

…

On December 23, 2016 1:08:51 PM EST, Gregory Maxwell ***@***.***> wrote: @TheBlueMatt not so! as it could skip processing when there are no peers requesting, and update caches any time it processes! I don't think it matters except in one case: Isolated mining node with no HB requesting peers, constructing the short block earlier will delay block validation which delays updating the create new block. I'm guessing the delay is <10ms, so probably inconsequential. But thought I'd bring it to your attention. -- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: #9375 (comment)

gmaxwell · 2016-12-23T22:27:38Z

src/net_processing.cpp

+
+    connman->ForEachNode([this, &cmpctblock, pindex, &msgMaker, fWitnessEnabled](CNode* pnode) {
+        // TODO: Avoid the repeated-serialization here
+        if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION)


I believe this needs a fDisconnect check.

TheBlueMatt · 2016-12-24T16:18:32Z

Squashed with no diff-tree.

theuni · 2016-12-29T20:15:24Z

src/validation.cpp

 {
    {
        LOCK(cs_main);
        for (const CBlockHeader& header : headers) {
-            if (!AcceptBlockHeader(header, state, chainparams, ppindex)) {
+            // cast away the ppindex-returns-const CBlockIndex - we're just assigning it to a CBlockIndex*


Hmm, this is non-obvious to a caller. And it's especially bad that one of them relies on the const not actually being const :(

Maybe return an updated index, or null in the case of failure? Or return a pair?

Really? I dont see anything non-obvious here? The caller passes in a reference to a const CBlockIndex*, and the function sets that element to the new CBlockIndex*, ie it is returning a const CBlockIndex*, not editing a CBlockIndex which was passed in.

To avoid the const cast, you could do something like:

CBlockIndex* pindex = nullptr; if (!AcceptBlockHeader(header, state, chainparams, &pindex)) { return false; } *ppindex = pindex;

I prefer @ryanofsky's code.

OK, changed.

theuni · 2016-12-29T20:34:44Z

src/validation.cpp

@@ -3799,16 +3802,19 @@ bool LoadExternalBlockFile(const CChainParams& chainparams, FILE* fileIn, CDiskB
                    std::pair<std::multimap<uint256, CDiskBlockPos>::iterator, std::multimap<uint256, CDiskBlockPos>::iterator> range = mapBlocksUnknownParent.equal_range(head);
                    while (range.first != range.second) {
                        std::multimap<uint256, CDiskBlockPos>::iterator it = range.first;
-                        if (ReadBlockFromDisk(block, it->second, chainparams.GetConsensus()))


Maybe teach ReadBlockFromDisk to read to a shared_ptr reference to avoid the temptation?

Or just use a new precursiveblock here with limited scope

theuni · 2016-12-29T20:53:25Z

src/net_processing.cpp

+
+    LOCK(cs_main);
+    bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, Params().GetConsensus());
+    uint256 hashBlock(pblock->GetHash());


How about generating a set of to-be-sent nodes here, with cs_main scope-locked. Then pass the set into ForEachNode and send only to the matches?

That saves cs_main (or whatever lock) from being held for the duration, and opens the door to deduped sending.

AFICT the only difference is that we can skip calling connman->PushMessage....is connman->PushMessage slow enough to care?

You can avoid calling it for each node by iterating mapNodeState for the set up front.

One PushMessage is no concern, but avoiding it for the entire ForEachNode would be nice.

Actually, nevermind. We can improve CConnman's interface for this kind of thing later.

Yup, that was my (too heavily implied) point :).

theuni · 2016-12-29T21:04:09Z

src/net_processing.cpp

@@ -760,6 +764,12 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std:
    bool fWitnessEnabled = IsWitnessEnabled(pindex->pprev, Params().GetConsensus());
    uint256 hashBlock(pblock->GetHash());

+    {


Make this a member of PeerLogicValidation, then you can use a weak_ptr for the global, and I think no locks are needed

I'm confused as to exactly what you're suggesting here. I don't particularly want a weak_ptr because I do want to hold the latest block for responses for a while after the block is connected/broadcast.

These are the types of changes that make me nervous about shared_ptrs, because it gets easy to start stashing things that have no obvious lifetime constraint.

I was suggesting that you hold the shared_ptr in the PeerLogicValidation class. You then use a global weak_ptr which is overwritten by each new block. Then at any random time you can use std::shared_ptr<const CBlock> bestblock = shared_ptrglobal_last_block.lock().

That way you can extend the lifetime of the whatever block is current, but it's also obvious when it will be destroyed (with the PeerLogicValidation destruction).

Hmm...does this make it that much more obvious? We still have a shared_ptr which is overwritten in NewPoWValidBlock and other sites which take references to it to extend its lifetime. I can add a comment noting that users MUST NOT extend the lifetime significantly, would that be sufficient to address your concern?

theuni · 2016-12-29T21:11:56Z

src/net_processing.cpp

+    }
+    LOCK(cs_main);
+    CNetMsgMaker msgMaker(pfrom->GetSendVersion());
+    int nSendFlags = State(pfrom->GetId())->fWantsCmpctWitness ? 0 : SERIALIZE_TRANSACTION_NO_WITNESS;


No need to stay locked for PushMessage

Is it slow enough to matter? After #9414 (and some std::atomic usage) we need to remove the cs_main lock completely here to make getblocktxn for the current head cs_main-free.

TheBlueMatt · 2016-12-29T22:18:19Z

Fixed one of @theuni's comments and rebased. Diff is https://0bin.net/paste/lZfeK4vg0ZdebkqX#AJKsFyZ3Ai1oiaRiUt-1S0tjFK9wrZ70aP74IepwbsY

TheBlueMatt · 2017-01-12T20:17:11Z

Address all of @sipa's comments, and added a comment to (hopefully) appease @theuni.

morcos · 2017-01-12T21:38:28Z

OK, I'm going for number 5!

tested ACK c1ae4fc
utACK 73666ad

To reemphasize, the caching in this PR provides a huge performance win.

theuni · 2017-01-13T03:08:11Z

@TheBlueMatt Thanks for adding the comment. utACK 73666ad.

sdaftuar · 2017-01-13T15:58:01Z

utACK 73666ad

sipa · 2017-01-13T20:41:08Z

utACK 73666ad

sipa · 2017-01-13T18:48:42Z

src/net_processing.cpp

@@ -754,10 +754,11 @@ void PeerLogicValidation::SyncTransaction(const CTransaction& tx, const CBlockIn

 static CCriticalSection cs_most_recent_block;
 static std::shared_ptr<const CBlock> most_recent_block;
+static std::shared_ptr<CBlockHeaderAndShortTxIDs> most_recent_compact_block;


Can this be a std::make_shared<const CBlockHeaderAndShortTxIDS> (slightly easier to reason about thread safety when shared_ptr objects are const).

sipa · 2017-01-13T20:34:21Z

src/net_processing.cpp

+                    LOCK(cs_most_recent_block);
+                    if (most_recent_block_hash == pBestIndex->GetBlockHash()) {
+                        if (state.fWantsCmpctWitness)
+                            connman.PushMessage(pto, msgMaker.Make(nSendFlags, NetMsgType::CMPCTBLOCK, *most_recent_compact_block));


Idea for follow-up PR: cache the CSerializedNetMsg instead of the CBlockHeaderAndShortTxIDs (smaller and faster to process)?

TheBlueMatt · 2017-01-13T20:50:45Z

Pushed one (hopefully last) commit which only adds two const's to net_processing at @sipa's request.

sipa · 2017-01-13T21:14:41Z

src/net_processing.cpp

 static uint256 most_recent_block_hash;

 void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std::shared_ptr<const CBlock>& pblock) {
-    std::shared_ptr<CBlockHeaderAndShortTxIDs> pcmpctblock = std::make_shared<CBlockHeaderAndShortTxIDs> (*pblock, true);
+    std::shared_ptr<const CBlockHeaderAndShortTxIDs> pcmpctblock = std::make_shared<CBlockHeaderAndShortTxIDs> (*pblock, true);


Also add const to the make_shared parameter?

Ok, did so and squashed it into the previous commit.

02ee4eb Make most_recent_compact_block a pointer to a const (Matt Corallo) 73666ad Add comment to describe callers to ActivateBestChain (Matt Corallo) 962f7f0 Call ActivateBestChain without cs_main/with most_recent_block (Matt Corallo) 0df777d Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders (Matt Corallo) c1ae4fc Avoid holding cs_most_recent_block while calling ReadBlockFromDisk (Matt Corallo) 9eb67f5 Ensure we meet the BIP 152 old-relay-types response requirements (Matt Corallo) 5749a85 Cache most-recently-connected compact block (Matt Corallo) 9eaec08 Cache most-recently-announced block's shared_ptr (Matt Corallo) c802092 Relay compact block messages prior to full block connection (Matt Corallo) 6987219 Add a CValidationInterface::NewPoWValidBlock callback (Matt Corallo) 180586f Call AcceptBlock with the block's shared_ptr instead of CBlock& (Matt Corallo) 8baaba6 [qa] Avoid race in preciousblock test. (Matt Corallo) 9a0b2f4 [qa] Make compact blocks test construction using fetch methods (Matt Corallo) 8017547 Make CBlockIndex*es in net_processing const (Matt Corallo)

rebroad · 2017-01-17T14:21:45Z

src/net_processing.cpp

+                            a_recent_block = most_recent_block;
+                        }
+                        CValidationState dummy;
+                        ActivateBestChain(dummy, Params(), a_recent_block);


I'm struggling to understand why this is really needed, unless it is here to cater for a re-org that goes back more than 30 days as the logic below allows blocks to be sent that are not in the main chain but are less than 30 days old.

…connection 02ee4eb Make most_recent_compact_block a pointer to a const (Matt Corallo) 73666ad Add comment to describe callers to ActivateBestChain (Matt Corallo) 962f7f0 Call ActivateBestChain without cs_main/with most_recent_block (Matt Corallo) 0df777d Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders (Matt Corallo) c1ae4fc Avoid holding cs_most_recent_block while calling ReadBlockFromDisk (Matt Corallo) 9eb67f5 Ensure we meet the BIP 152 old-relay-types response requirements (Matt Corallo) 5749a85 Cache most-recently-connected compact block (Matt Corallo) 9eaec08 Cache most-recently-announced block's shared_ptr (Matt Corallo) c802092 Relay compact block messages prior to full block connection (Matt Corallo) 6987219 Add a CValidationInterface::NewPoWValidBlock callback (Matt Corallo) 180586f Call AcceptBlock with the block's shared_ptr instead of CBlock& (Matt Corallo) 8baaba6 [qa] Avoid race in preciousblock test. (Matt Corallo) 9a0b2f4 [qa] Make compact blocks test construction using fetch methods (Matt Corallo) 8017547 Make CBlockIndex*es in net_processing const (Matt Corallo)

* Merge bitcoin#8068: Compact Blocks 48efec8 Fix some minor compact block issues that came up in review (Matt Corallo) ccd06b9 Elaborate bucket size math (Pieter Wuille) 0d4cb48 Use vTxHashes to optimize InitData significantly (Matt Corallo) 8119026 Provide a flat list of txid/terators to txn in CTxMemPool (Matt Corallo) 678ee97 Add BIP 152 to implemented BIPs list (Matt Corallo) 56ba516 Add reconstruction debug logging (Matt Corallo) 2f34a2e Get our "best three" peers to announce blocks using cmpctblocks (Matt Corallo) 927f8ee Add ability to fetch CNode by NodeId (Matt Corallo) d25cd3e Add receiver-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 9c837d5 Add sender-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 00c4078 Add protocol messages for short-ids blocks (Matt Corallo) e3b2222 Add some blockencodings tests (Matt Corallo) f4f8f14 Add TestMemPoolEntryHelper::FromTx version for CTransaction (Matt Corallo) 85ad31e Add partial-block block encodings API (Matt Corallo) 5249dac Add COMPACTSIZE wrapper similar to VARINT for serialization (Matt Corallo) cbda71c Move context-required checks from CheckBlockHeader to Contextual... (Matt Corallo) 7c29ec9 If AcceptBlockHeader returns true, pindex will be set. (Matt Corallo) 96806c3 Stop trimming when mapTx is empty (Pieter Wuille) * Merge bitcoin#8408: Prevent fingerprinting, disk-DoS with compact blocks 1d06e49 Ignore CMPCTBLOCK messages for pruned blocks (Suhas Daftuar) 1de2a46 Ignore GETBLOCKTXN requests for unknown blocks (Suhas Daftuar) * Merge bitcoin#8418: Add tests for compact blocks 45c7ddd Add p2p test for BIP 152 (compact blocks) (Suhas Daftuar) 9a22a6c Add support for compactblocks to mininode (Suhas Daftuar) a8689fd Tests: refactor compact size serialization in mininode (Suhas Daftuar) 9c8593d Implement SipHash in Python (Pieter Wuille) 56c87e9 Allow changing BIP9 parameters on regtest (Suhas Daftuar) * Merge bitcoin#8505: Trivial: Fix typos in various files 1aacfc2 various typos (leijurv) * Merge bitcoin#8449: [Trivial] Do not shadow local variable, cleanup a159f25 Remove redundand (and shadowing) declaration (Pavel Janík) cce3024 Do not shadow local variable, cleanup (Pavel Janík) * Merge bitcoin#8739: [qa] Fix broken sendcmpct test in p2p-compactblocks.py 157254a Fix broken sendcmpct test in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8854: [qa] Fix race condition in p2p-compactblocks test b5fd666 [qa] Fix race condition in p2p-compactblocks test (Suhas Daftuar) * Merge bitcoin#8393: Support for compact blocks together with segwit 27acfc1 [qa] Update p2p-compactblocks.py for compactblocks v2 (Suhas Daftuar) 422fac6 [qa] Add support for compactblocks v2 to mininode (Suhas Daftuar) f5b9b8f [qa] Fix bug in mininode witness deserialization (Suhas Daftuar) 6aa28ab Use cmpctblock type 2 for segwit-enabled transfer (Pieter Wuille) be7555f Fix overly-prescriptive p2p-segwit test for new fetch logic (Matt Corallo) 06128da Make GetFetchFlags always request witness objects from witness peers (Matt Corallo) * Merge bitcoin#8882: [qa] Fix race conditions in p2p-compactblocks.py and sendheaders.py b55d941 [qa] Fix race condition in sendheaders.py (Suhas Daftuar) 6976db2 [qa] Another attempt to fix race condition in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8904: [qa] Fix compact block shortids for a test case 4cdece4 [qa] Fix compact block shortids for a test case (Dagur Valberg Johannsson) * Merge bitcoin#8637: Compact Block Tweaks (rebase of bitcoin#8235) 3ac6de0 Align constant names for maximum compact block / blocktxn depth (Pieter Wuille) b2e93a3 Add cmpctblock to debug help list (instagibbs) fe998e9 More agressively filter compact block requests (Matt Corallo) 02a337d Dont remove a "preferred" cmpctblock peer if they provide a block (Matt Corallo) * Merge bitcoin#8975: Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ 6f2f639 Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ (Jorge Timón) * Merge bitcoin#8968: Don't hold cs_main when calling ProcessNewBlock from a cmpctblock 72ca7d9 Don't hold cs_main when calling ProcessNewBlock from a cmpctblock (Matt Corallo) * Merge bitcoin#8995: Add missing cs_main lock to ::GETBLOCKTXN processing dfe7906 Add missing cs_main lock to ::GETBLOCKTXN processing (Matt Corallo) * Merge bitcoin#8515: A few mempool removal optimizations 0334430 Add some missing includes (Pieter Wuille) 4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille) 51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille) f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille) * Merge bitcoin#9026: Fix handling of invalid compact blocks d4833ff Bump the protocol version to distinguish new banning behavior. (Suhas Daftuar) 88c3549 Fix compact block handling to not ban if block is invalid (Suhas Daftuar) c93beac [qa] Test that invalid compactblocks don't result in ban (Suhas Daftuar) * Merge bitcoin#9039: Various serialization simplifcations and optimizations d59a518 Use fixed preallocation instead of costly GetSerializeSize (Pieter Wuille) 25a211a Add optimized CSizeComputer serializers (Pieter Wuille) a2929a2 Make CSerAction's ForRead() constexpr (Pieter Wuille) a603925 Avoid -Wshadow errors (Pieter Wuille) 5284721 Get rid of nType and nVersion (Pieter Wuille) 657e05a Make GetSerializeSize a wrapper on top of CSizeComputer (Pieter Wuille) fad9b66 Make nType and nVersion private and sometimes const (Pieter Wuille) c2c5d42 Make streams' read and write return void (Pieter Wuille) 50e8a9c Remove unused ReadVersion and WriteVersion (Pieter Wuille) * Merge bitcoin#9058: Fixes for p2p-compactblocks.py test timeouts on travis (bitcoin#8842) dac53b5 Modify getblocktxn handler not to drop requests for old blocks (Russell Yanofsky) 55bfddc [qa] Fix stale data bug in test_compactblocks_not_at_tip (Russell Yanofsky) 47e9659 [qa] Fix bug in compactblocks v2 merge (Russell Yanofsky) * Merge bitcoin#9160: [trivial] Fix hungarian variable name ec34648 [trivial] Fix hungarian variable name (Russell Yanofsky) * Merge bitcoin#9159: [qa] Wait for specific block announcement in p2p-compactblocks dfa44d1 [qa] Wait for specific block announcement in p2p-compactblocks (Russell Yanofsky) * Merge bitcoin#9125: Make CBlock a vector of shared_ptr of CTransactions b4e4ba4 Introduce convenience type CTransactionRef (Pieter Wuille) 1662b43 Make CBlock::vtx a vector of shared_ptr<CTransaction> (Pieter Wuille) da60506 Add deserializing constructors to CTransaction and CMutableTransaction (Pieter Wuille) 0e85204 Add serialization for unique_ptr and shared_ptr (Pieter Wuille) * Merge bitcoin#8872: Remove block-request logic from INV message processing 037159c Remove block-request logic from INV message processing (Matt Corallo) 3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo) d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews) * Merge bitcoin#9199: Always drop the least preferred HB peer when adding a new one. ca8549d Always drop the least preferred HB peer when adding a new one. (Gregory Maxwell) * Merge bitcoin#9233: Fix some typos 15fa95d Fix some typos (fsb4000) * Merge bitcoin#9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) 76faa3c Rename the remaining main.{h,cpp} to validation.{h,cpp} (Matt Corallo) e736772 Move network-msg-processing code out of main to its own file (Matt Corallo) 87c35f5 Remove orphan state wipe from UnloadBlockIndex. (Matt Corallo) * Merge bitcoin#9014: Fix block-connection performance regression dd0df81 Document ConnectBlock connectTrace postconditions (Matt Corallo) 2d6e561 Switch pblock in ProcessNewBlock to a shared_ptr (Matt Corallo) 2736c44 Make the optional pblock in ActivateBestChain a shared_ptr (Matt Corallo) ae4db44 Create a shared_ptr for the block we're connecting in ActivateBCS (Matt Corallo) fd9d890 Keep blocks as shared_ptrs, instead of copying txn in ConnectTip (Matt Corallo) 6fdd43b Add struct to track block-connect-time-generated info for callbacks (Matt Corallo) * Merge bitcoin#9240: Remove txConflicted a874ab5 remove internal tracking of mempool conflicts for reporting to wallet (Alex Morcos) bf663f8 remove external usage of mempool conflict tracking (Alex Morcos) * Merge bitcoin#9344: Do not run functions with necessary side-effects in assert() da9cdd2 Do not run functions with necessary side-effects in assert() (Gregory Maxwell) * Merge bitcoin#9273: Remove unused CDiskBlockPos* argument from ProcessNewBlock a13fa4c Remove unused CDiskBlockPos* argument from ProcessNewBlock (Matt Corallo) * Merge bitcoin#9352: Attempt reconstruction from all compact block announcements 813ede9 [qa] Update compactblocks test for multi-peer reconstruction (Suhas Daftuar) 7017298 Allow compactblock reconstruction when block is in flight (Suhas Daftuar) * Merge bitcoin#9252: Release cs_main before calling ProcessNewBlock, or processing headers (cmpctblock handling) bd02bdd Release cs_main before processing cmpctblock as header (Suhas Daftuar) 680b0c0 Release cs_main before calling ProcessNewBlock (cmpctblock handling) (Suhas Daftuar) * Merge bitcoin#9283: A few more CTransactionRef optimizations 91335ba Remove unused MakeTransactionRef overloads (Pieter Wuille) 6713f0f Make FillBlock consume txn_available to avoid shared_ptr copies (Pieter Wuille) 62607d7 Convert COrphanTx to keep a CTransactionRef (Pieter Wuille) c44e4c4 Make AcceptToMemoryPool take CTransactionRef (Pieter Wuille) * Merge bitcoin#9375: Relay compact block messages prior to full block connection 02ee4eb Make most_recent_compact_block a pointer to a const (Matt Corallo) 73666ad Add comment to describe callers to ActivateBestChain (Matt Corallo) 962f7f0 Call ActivateBestChain without cs_main/with most_recent_block (Matt Corallo) 0df777d Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders (Matt Corallo) c1ae4fc Avoid holding cs_most_recent_block while calling ReadBlockFromDisk (Matt Corallo) 9eb67f5 Ensure we meet the BIP 152 old-relay-types response requirements (Matt Corallo) 5749a85 Cache most-recently-connected compact block (Matt Corallo) 9eaec08 Cache most-recently-announced block's shared_ptr (Matt Corallo) c802092 Relay compact block messages prior to full block connection (Matt Corallo) 6987219 Add a CValidationInterface::NewPoWValidBlock callback (Matt Corallo) 180586f Call AcceptBlock with the block's shared_ptr instead of CBlock& (Matt Corallo) 8baaba6 [qa] Avoid race in preciousblock test. (Matt Corallo) 9a0b2f4 [qa] Make compact blocks test construction using fetch methods (Matt Corallo) 8017547 Make CBlockIndex*es in net_processing const (Matt Corallo) * Merge bitcoin#9486: Make peer=%d log prints consistent e6111b2 Make peer id logging consistent ("peer=%d" instead of "peer %d") (Matt Corallo) * Merge bitcoin#9400: Set peers as HB peers upon full block validation d4781ac Set peers as HB peers upon full block validation (Gregory Sanders) * Merge bitcoin#9499: Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction c594580 Add braces around AddToCompactExtraTransactions (Matt Corallo) 1ccfe9b Clarify comment about mempool/extra conflicts (Matt Corallo) fac4c78 Make PartiallyDownloadedBlock::InitData's second param const (Matt Corallo) b55b416 Add extra_count lower bound to compact reconstruction debug print (Matt Corallo) 863edb4 Consider all (<100k memusage) txn for compact-block-extra-txn cache (Matt Corallo) 7f8c8ca Consider all orphan txn for compact-block-extra-txn cache (Matt Corallo) 93380c5 Use replaced transactions in compact block reconstruction (Matt Corallo) 1531652 Keep shared_ptrs to recently-replaced txn for compact blocks (Matt Corallo) edded80 Make ATMP optionally return the CTransactionRefs it replaced (Matt Corallo) c735540 Move ORPHAN constants from validation.h to net_processing.h (Matt Corallo) * Merge bitcoin#9587: Do not shadow local variable named `tx`. 44f2baa Do not shadow local variable named `tx`. (Pavel Janík) * Merge bitcoin#9510: [trivial] Fix typos in comments cc16d99 [trivial] Fix typos in comments (practicalswift) * Merge bitcoin#9604: [Trivial] add comment about setting peer as HB peer. dd5b011 [Trivial] add comment about setting peer as HB peer. (John Newbery) * Fix using of AcceptToMemoryPool in PrivateSend code * add `override` * fSupportsDesiredCmpctVersion * bring back tx ressurection in DisconnectTip * Fix delayed headers * Remove unused CConnman::FindNode overload * Fix typos and comments * Fix minor code differences * Don't use rejection cache for corrupted transactions Partly based on bitcoin#8525 * Backport missed cs_main locking changes Missed from bitcoin@58a215c * Backport missed comments and mapBlockSource.emplace call Missed from two commits: bitcoin@88c3549 bitcoin@7c98ce5 * Add CheckPeerHeaders() helper and check in (nCount == 0) too

48efec8 Fix some minor compact block issues that came up in review (Matt Corallo) ccd06b9 Elaborate bucket size math (Pieter Wuille) 0d4cb48 Use vTxHashes to optimize InitData significantly (Matt Corallo) 8119026 Provide a flat list of txid/terators to txn in CTxMemPool (Matt Corallo) 678ee97 Add BIP 152 to implemented BIPs list (Matt Corallo) 56ba516 Add reconstruction debug logging (Matt Corallo) 2f34a2e Get our "best three" peers to announce blocks using cmpctblocks (Matt Corallo) 927f8ee Add ability to fetch CNode by NodeId (Matt Corallo) d25cd3e Add receiver-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 9c837d5 Add sender-side protocol implementation for CMPCTBLOCK stuff (Matt Corallo) 00c4078 Add protocol messages for short-ids blocks (Matt Corallo) e3b2222 Add some blockencodings tests (Matt Corallo) f4f8f14 Add TestMemPoolEntryHelper::FromTx version for CTransaction (Matt Corallo) 85ad31e Add partial-block block encodings API (Matt Corallo) 5249dac Add COMPACTSIZE wrapper similar to VARINT for serialization (Matt Corallo) cbda71c Move context-required checks from CheckBlockHeader to Contextual... (Matt Corallo) 7c29ec9 If AcceptBlockHeader returns true, pindex will be set. (Matt Corallo) 96806c3 Stop trimming when mapTx is empty (Pieter Wuille) * Merge bitcoin#8408: Prevent fingerprinting, disk-DoS with compact blocks 1d06e49 Ignore CMPCTBLOCK messages for pruned blocks (Suhas Daftuar) 1de2a46 Ignore GETBLOCKTXN requests for unknown blocks (Suhas Daftuar) * Merge bitcoin#8418: Add tests for compact blocks 45c7ddd Add p2p test for BIP 152 (compact blocks) (Suhas Daftuar) 9a22a6c Add support for compactblocks to mininode (Suhas Daftuar) a8689fd Tests: refactor compact size serialization in mininode (Suhas Daftuar) 9c8593d Implement SipHash in Python (Pieter Wuille) 56c87e9 Allow changing BIP9 parameters on regtest (Suhas Daftuar) * Merge bitcoin#8505: Trivial: Fix typos in various files 1aacfc2 various typos (leijurv) * Merge bitcoin#8449: [Trivial] Do not shadow local variable, cleanup a159f25 Remove redundand (and shadowing) declaration (Pavel Janík) cce3024 Do not shadow local variable, cleanup (Pavel Janík) * Merge bitcoin#8739: [qa] Fix broken sendcmpct test in p2p-compactblocks.py 157254a Fix broken sendcmpct test in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8854: [qa] Fix race condition in p2p-compactblocks test b5fd666 [qa] Fix race condition in p2p-compactblocks test (Suhas Daftuar) * Merge bitcoin#8393: Support for compact blocks together with segwit 27acfc1 [qa] Update p2p-compactblocks.py for compactblocks v2 (Suhas Daftuar) 422fac6 [qa] Add support for compactblocks v2 to mininode (Suhas Daftuar) f5b9b8f [qa] Fix bug in mininode witness deserialization (Suhas Daftuar) 6aa28ab Use cmpctblock type 2 for segwit-enabled transfer (Pieter Wuille) be7555f Fix overly-prescriptive p2p-segwit test for new fetch logic (Matt Corallo) 06128da Make GetFetchFlags always request witness objects from witness peers (Matt Corallo) * Merge bitcoin#8882: [qa] Fix race conditions in p2p-compactblocks.py and sendheaders.py b55d941 [qa] Fix race condition in sendheaders.py (Suhas Daftuar) 6976db2 [qa] Another attempt to fix race condition in p2p-compactblocks.py (Suhas Daftuar) * Merge bitcoin#8904: [qa] Fix compact block shortids for a test case 4cdece4 [qa] Fix compact block shortids for a test case (Dagur Valberg Johannsson) * Merge bitcoin#8637: Compact Block Tweaks (rebase of bitcoin#8235) 3ac6de0 Align constant names for maximum compact block / blocktxn depth (Pieter Wuille) b2e93a3 Add cmpctblock to debug help list (instagibbs) fe998e9 More agressively filter compact block requests (Matt Corallo) 02a337d Dont remove a "preferred" cmpctblock peer if they provide a block (Matt Corallo) * Merge bitcoin#8975: Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ 6f2f639 Chainparams: Trivial: In AppInit2(), s/Params()/chainparams/ (Jorge Timón) * Merge bitcoin#8968: Don't hold cs_main when calling ProcessNewBlock from a cmpctblock 72ca7d9 Don't hold cs_main when calling ProcessNewBlock from a cmpctblock (Matt Corallo) * Merge bitcoin#8995: Add missing cs_main lock to ::GETBLOCKTXN processing dfe7906 Add missing cs_main lock to ::GETBLOCKTXN processing (Matt Corallo) * Merge bitcoin#8515: A few mempool removal optimizations 0334430 Add some missing includes (Pieter Wuille) 4100499 Return shared_ptr<CTransaction> from mempool removes (Pieter Wuille) 51f2783 Make removed and conflicted arguments optional to remove (Pieter Wuille) f48211b Bypass removeRecursive in removeForReorg (Pieter Wuille) * Merge bitcoin#9026: Fix handling of invalid compact blocks d4833ff Bump the protocol version to distinguish new banning behavior. (Suhas Daftuar) 88c3549 Fix compact block handling to not ban if block is invalid (Suhas Daftuar) c93beac [qa] Test that invalid compactblocks don't result in ban (Suhas Daftuar) * Merge bitcoin#9039: Various serialization simplifcations and optimizations d59a518 Use fixed preallocation instead of costly GetSerializeSize (Pieter Wuille) 25a211a Add optimized CSizeComputer serializers (Pieter Wuille) a2929a2 Make CSerAction's ForRead() constexpr (Pieter Wuille) a603925 Avoid -Wshadow errors (Pieter Wuille) 5284721 Get rid of nType and nVersion (Pieter Wuille) 657e05a Make GetSerializeSize a wrapper on top of CSizeComputer (Pieter Wuille) fad9b66 Make nType and nVersion private and sometimes const (Pieter Wuille) c2c5d42 Make streams' read and write return void (Pieter Wuille) 50e8a9c Remove unused ReadVersion and WriteVersion (Pieter Wuille) * Merge bitcoin#9058: Fixes for p2p-compactblocks.py test timeouts on travis (bitcoin#8842) dac53b5 Modify getblocktxn handler not to drop requests for old blocks (Russell Yanofsky) 55bfddc [qa] Fix stale data bug in test_compactblocks_not_at_tip (Russell Yanofsky) 47e9659 [qa] Fix bug in compactblocks v2 merge (Russell Yanofsky) * Merge bitcoin#9160: [trivial] Fix hungarian variable name ec34648 [trivial] Fix hungarian variable name (Russell Yanofsky) * Merge bitcoin#9159: [qa] Wait for specific block announcement in p2p-compactblocks dfa44d1 [qa] Wait for specific block announcement in p2p-compactblocks (Russell Yanofsky) * Merge bitcoin#9125: Make CBlock a vector of shared_ptr of CTransactions b4e4ba4 Introduce convenience type CTransactionRef (Pieter Wuille) 1662b43 Make CBlock::vtx a vector of shared_ptr<CTransaction> (Pieter Wuille) da60506 Add deserializing constructors to CTransaction and CMutableTransaction (Pieter Wuille) 0e85204 Add serialization for unique_ptr and shared_ptr (Pieter Wuille) * Merge bitcoin#8872: Remove block-request logic from INV message processing 037159c Remove block-request logic from INV message processing (Matt Corallo) 3451203 [qa] Respond to getheaders and do not assume a getdata on inv (Matt Corallo) d768f15 [qa] Make comptool push blocks instead of relying on inv-fetch (mrbandrews) * Merge bitcoin#9199: Always drop the least preferred HB peer when adding a new one. ca8549d Always drop the least preferred HB peer when adding a new one. (Gregory Maxwell) * Merge bitcoin#9233: Fix some typos 15fa95d Fix some typos (fsb4000) * Merge bitcoin#9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) 76faa3c Rename the remaining main.{h,cpp} to validation.{h,cpp} (Matt Corallo) e736772 Move network-msg-processing code out of main to its own file (Matt Corallo) 87c35f5 Remove orphan state wipe from UnloadBlockIndex. (Matt Corallo) * Merge bitcoin#9014: Fix block-connection performance regression dd0df81 Document ConnectBlock connectTrace postconditions (Matt Corallo) 2d6e561 Switch pblock in ProcessNewBlock to a shared_ptr (Matt Corallo) 2736c44 Make the optional pblock in ActivateBestChain a shared_ptr (Matt Corallo) ae4db44 Create a shared_ptr for the block we're connecting in ActivateBCS (Matt Corallo) fd9d890 Keep blocks as shared_ptrs, instead of copying txn in ConnectTip (Matt Corallo) 6fdd43b Add struct to track block-connect-time-generated info for callbacks (Matt Corallo) * Merge bitcoin#9240: Remove txConflicted a874ab5 remove internal tracking of mempool conflicts for reporting to wallet (Alex Morcos) bf663f8 remove external usage of mempool conflict tracking (Alex Morcos) * Merge bitcoin#9344: Do not run functions with necessary side-effects in assert() da9cdd2 Do not run functions with necessary side-effects in assert() (Gregory Maxwell) * Merge bitcoin#9273: Remove unused CDiskBlockPos* argument from ProcessNewBlock a13fa4c Remove unused CDiskBlockPos* argument from ProcessNewBlock (Matt Corallo) * Merge bitcoin#9352: Attempt reconstruction from all compact block announcements 813ede9 [qa] Update compactblocks test for multi-peer reconstruction (Suhas Daftuar) 7017298 Allow compactblock reconstruction when block is in flight (Suhas Daftuar) * Merge bitcoin#9252: Release cs_main before calling ProcessNewBlock, or processing headers (cmpctblock handling) bd02bdd Release cs_main before processing cmpctblock as header (Suhas Daftuar) 680b0c0 Release cs_main before calling ProcessNewBlock (cmpctblock handling) (Suhas Daftuar) * Merge bitcoin#9283: A few more CTransactionRef optimizations 91335ba Remove unused MakeTransactionRef overloads (Pieter Wuille) 6713f0f Make FillBlock consume txn_available to avoid shared_ptr copies (Pieter Wuille) 62607d7 Convert COrphanTx to keep a CTransactionRef (Pieter Wuille) c44e4c4 Make AcceptToMemoryPool take CTransactionRef (Pieter Wuille) * Merge bitcoin#9375: Relay compact block messages prior to full block connection 02ee4eb Make most_recent_compact_block a pointer to a const (Matt Corallo) 73666ad Add comment to describe callers to ActivateBestChain (Matt Corallo) 962f7f0 Call ActivateBestChain without cs_main/with most_recent_block (Matt Corallo) 0df777d Use a temp pindex to avoid a const_cast in ProcessNewBlockHeaders (Matt Corallo) c1ae4fc Avoid holding cs_most_recent_block while calling ReadBlockFromDisk (Matt Corallo) 9eb67f5 Ensure we meet the BIP 152 old-relay-types response requirements (Matt Corallo) 5749a85 Cache most-recently-connected compact block (Matt Corallo) 9eaec08 Cache most-recently-announced block's shared_ptr (Matt Corallo) c802092 Relay compact block messages prior to full block connection (Matt Corallo) 6987219 Add a CValidationInterface::NewPoWValidBlock callback (Matt Corallo) 180586f Call AcceptBlock with the block's shared_ptr instead of CBlock& (Matt Corallo) 8baaba6 [qa] Avoid race in preciousblock test. (Matt Corallo) 9a0b2f4 [qa] Make compact blocks test construction using fetch methods (Matt Corallo) 8017547 Make CBlockIndex*es in net_processing const (Matt Corallo) * Merge bitcoin#9486: Make peer=%d log prints consistent e6111b2 Make peer id logging consistent ("peer=%d" instead of "peer %d") (Matt Corallo) * Merge bitcoin#9400: Set peers as HB peers upon full block validation d4781ac Set peers as HB peers upon full block validation (Gregory Sanders) * Merge bitcoin#9499: Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction c594580 Add braces around AddToCompactExtraTransactions (Matt Corallo) 1ccfe9b Clarify comment about mempool/extra conflicts (Matt Corallo) fac4c78 Make PartiallyDownloadedBlock::InitData's second param const (Matt Corallo) b55b416 Add extra_count lower bound to compact reconstruction debug print (Matt Corallo) 863edb4 Consider all (<100k memusage) txn for compact-block-extra-txn cache (Matt Corallo) 7f8c8ca Consider all orphan txn for compact-block-extra-txn cache (Matt Corallo) 93380c5 Use replaced transactions in compact block reconstruction (Matt Corallo) 1531652 Keep shared_ptrs to recently-replaced txn for compact blocks (Matt Corallo) edded80 Make ATMP optionally return the CTransactionRefs it replaced (Matt Corallo) c735540 Move ORPHAN constants from validation.h to net_processing.h (Matt Corallo) * Merge bitcoin#9587: Do not shadow local variable named `tx`. 44f2baa Do not shadow local variable named `tx`. (Pavel Janík) * Merge bitcoin#9510: [trivial] Fix typos in comments cc16d99 [trivial] Fix typos in comments (practicalswift) * Merge bitcoin#9604: [Trivial] add comment about setting peer as HB peer. dd5b011 [Trivial] add comment about setting peer as HB peer. (John Newbery) * Fix using of AcceptToMemoryPool in PrivateSend code * add `override` * fSupportsDesiredCmpctVersion * bring back tx ressurection in DisconnectTip * Fix delayed headers * Remove unused CConnman::FindNode overload * Fix typos and comments * Fix minor code differences * Don't use rejection cache for corrupted transactions Partly based on bitcoin#8525 * Backport missed cs_main locking changes Missed from bitcoin/bitcoin@58a215c * Backport missed comments and mapBlockSource.emplace call Missed from two commits: bitcoin/bitcoin@88c3549 bitcoin/bitcoin@7c98ce5 * Add CheckPeerHeaders() helper and check in (nCount == 0) too

fanquake added P2P Validation labels Dec 19, 2016

TheBlueMatt force-pushed the 2016-12-compact-fast-relay branch from b7231e5 to 0e6f738 Compare December 19, 2016 08:50

rebroad reviewed Dec 21, 2016

View reviewed changes

morcos reviewed Dec 21, 2016

View reviewed changes

TheBlueMatt force-pushed the 2016-12-compact-fast-relay branch from d85513e to 5bc61fa Compare December 22, 2016 08:33

jonasschnelli added this to the 0.14.0 milestone Dec 22, 2016

gmaxwell reviewed Dec 23, 2016

View reviewed changes

TheBlueMatt force-pushed the 2016-12-compact-fast-relay branch from a3f4608 to f7e635b Compare December 24, 2016 16:18

TheBlueMatt mentioned this pull request Dec 24, 2016

Stop Using cs_main for CNodeState/State() #9419

Closed

theuni reviewed Dec 29, 2016

View reviewed changes

TheBlueMatt force-pushed the 2016-12-compact-fast-relay branch from f7e635b to 8022035 Compare December 29, 2016 22:16

Add comment to describe callers to ActivateBestChain

73666ad

sipa reviewed Jan 13, 2017

View reviewed changes

Make most_recent_compact_block a pointer to a const

02ee4eb

TheBlueMatt force-pushed the 2016-12-compact-fast-relay branch from 38bcdf7 to 02ee4eb Compare January 13, 2017 21:28

sipa merged commit 02ee4eb into bitcoin:master Jan 13, 2017

rebroad reviewed Jan 17, 2017

View reviewed changes

codablock mentioned this pull request Feb 8, 2018

Backports from compact block related Bitcoin PRs dashpay/dash#1911

Merged

gladcow mentioned this pull request Mar 1, 2018

Backport compact blocks functionality from bitcoin dashpay/dash#1966

Merged

adamjonas mentioned this pull request Dec 16, 2020

Don't request compact blocks when blocksonly=1 #8902

Closed

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Relay compact block messages prior to full block connection #9375

Relay compact block messages prior to full block connection #9375

Uh oh!

Conversation

TheBlueMatt commented Dec 19, 2016

Uh oh!

gmaxwell commented Dec 19, 2016

Uh oh!

TheBlueMatt commented Dec 19, 2016 via email

Uh oh!

gmaxwell commented Dec 19, 2016

Uh oh!

morcos commented Dec 21, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

morcos commented Dec 21, 2016

Uh oh!

TheBlueMatt commented Dec 22, 2016

Uh oh!

gmaxwell commented Dec 22, 2016

Uh oh!

morcos commented Dec 22, 2016

Uh oh!

TheBlueMatt commented Dec 22, 2016

Uh oh!

gmaxwell commented Dec 23, 2016

Uh oh!

TheBlueMatt commented Dec 23, 2016 via email

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Dec 24, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!