net, refactor: net_processing, add `ProcessCompactBlockTxns` #26969

brunoerg · 2023-01-25T21:11:30Z

When processing CMPCTBLOCK message, at some moments we can need to process compact block txns / BLOCKTXN, since all messages are handled by ProcessMessage, so we call ProcessMessage all over again.

bitcoin/src/net_processing.cpp

Lines 4331 to 4348 in ab98673

    
               if (fAlreadyInFlight) { 
        
                   // We requested this block, but its far into the future, so our 
        
                   // mempool will probably be useless - request the block normally 
        
                   std::vector<CInv> vInv(1); 
        
                   vInv[0] = CInv(MSG_BLOCK | GetFetchFlags(*peer), cmpctblock.header.GetHash()); 
        
                   m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv)); 
        
                   return; 
        
               } else { 
        
                   // If this was an announce-cmpctblock, we want the same treatment as a header message 
        
                   fRevertToHeaderProcessing = true; 
        
               } 
        
           } 
        
           } // cs_main 
        
           if (fProcessBLOCKTXN) { 
        
               return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, time_received, interruptMsgProc); 
        
           }

This PR creates a function called ProcessCompactBlockTxns to process it to avoid calling ProcessMessage for it - this function is also called when processing BLOCKTXN msg.

DrahtBot · 2023-01-25T21:11:32Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	instagibbs, ajtowns, achow101
Concept ACK	Sjors
Stale ACK	dergoegge

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27826 (validation: log which peer sent us a header by Sjors)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

dergoegge · 2023-01-25T21:23:18Z

Concept ACK

src/net_processing.cpp

brunoerg · 2023-01-26T16:01:02Z

Force-pushed addressing @ajtowns's suggestion.

src/net_processing.cpp

brunoerg · 2023-01-27T18:06:29Z

Rebased

src/net_processing.cpp

brunoerg · 2023-03-02T17:51:18Z

Force-pushed addressing #26969 (comment) (@dergoegge)

brunoerg · 2023-03-21T20:32:53Z

Rebased

ajtowns · 2023-06-06T20:02:32Z

ACK 355bc6098a7373feb1d59f9651a79e1477d22243

src/net_processing.cpp

instagibbs · 2023-06-07T14:46:13Z

re ACK 686629d

only differences beyond rebase is suggested changes for ProcessCompactBlockTxns argument ordering and const-ness of BlockTransactions arg.

brunoerg · 2023-06-07T14:46:56Z

Force-pushed addressing #26969 (comment)

ajtowns · 2023-06-08T00:04:41Z

re ACK 686629d2be5545ef59cf0e97f4f3a74c6cde2efa

dergoegge

Code review ACK 686629d2be5545ef59cf0e97f4f3a74c6cde2efa

brunoerg · 2023-06-12T13:30:56Z

Rebased

instagibbs · 2023-06-12T13:34:43Z

reACK 89478af

ajtowns

reACK 89478af481b2e6d4652b2fe50ff5fe5b92cbd69e

src/net_processing.cpp

When processing `CMPCTBLOCK` message, at some moments we can need to process cmpct block txns, since all messages are handled by ProcessMessage, we call ProcessMessage all over again. For this reason, it creates a function called `ProcessCompactBlockTxns` to process it.

brunoerg · 2023-06-15T15:24:43Z

Force-pushed addressing #26969 (comment)

instagibbs · 2023-06-15T15:50:05Z

reACK 77d6d89

ajtowns · 2023-06-23T14:28:25Z

utACK 77d6d89

achow101 · 2023-06-23T22:10:56Z

ACK 77d6d89

Verified diff is primarily a moveonly that removes the recursion for BLOCKTXN.

…ctBlock…

DrahtBot added the P2P label Jan 25, 2023

ajtowns reviewed Jan 26, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

This was referenced Jan 26, 2023

refactor: Add BlockManager getters #26900

Merged

refactor: Make m_mempool optional in PeerManager #26247

Closed

Remove almost all blockstorage globals #25781

Merged

brunoerg changed the title ~~net: net_processing, add ProcessBlockTxnMessage~~ net: net_processing, add ProcessCompactBlockTxns Jan 26, 2023

brunoerg changed the title ~~net: net_processing, add ProcessCompactBlockTxns~~ net: net_processing, add ProcessCompactBlockTxns Jan 26, 2023

brunoerg force-pushed the 2023-01-netprocessing-txn branch from 3e95e62 to c17e71b Compare January 26, 2023 16:19

maflcko reviewed Jan 26, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

brunoerg force-pushed the 2023-01-netprocessing-txn branch from c17e71b to 9001051 Compare January 26, 2023 17:11

DrahtBot added the Needs rebase label Jan 27, 2023

brunoerg force-pushed the 2023-01-netprocessing-txn branch from 9001051 to fd27f29 Compare January 27, 2023 18:05

DrahtBot removed the Needs rebase label Jan 27, 2023

naumenkogs reviewed Feb 2, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Feb 2, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

naumenkogs reviewed Feb 2, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

brunoerg changed the title ~~net: net_processing, add ProcessCompactBlockTxns~~ net, refactor: net_processing, add ProcessCompactBlockTxns Feb 2, 2023

brunoerg force-pushed the 2023-01-netprocessing-txn branch 2 times, most recently from 1f7f871 to 3cd4578 Compare February 2, 2023 19:28

dergoegge reviewed Mar 1, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

brunoerg force-pushed the 2023-01-netprocessing-txn branch from 3cd4578 to e57f912 Compare March 2, 2023 17:51

DrahtBot mentioned this pull request Mar 19, 2023

Log new headers #27278

Merged

DrahtBot added the Needs rebase label Mar 21, 2023

brunoerg force-pushed the 2023-01-netprocessing-txn branch from e57f912 to fe4ad5f Compare March 21, 2023 20:30

DrahtBot mentioned this pull request Jun 5, 2023

validation: log which peer sent us a header #27826

Merged

dergoegge reviewed Jun 7, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

brunoerg force-pushed the 2023-01-netprocessing-txn branch from 355bc60 to 686629d Compare June 7, 2023 14:43

DrahtBot requested a review from ajtowns June 7, 2023 14:46

DrahtBot mentioned this pull request Jun 7, 2023

p2p: Stop relaying non-mempool txs #27625

Merged

DrahtBot removed the request for review from ajtowns June 8, 2023 00:04

dergoegge approved these changes Jun 8, 2023

View reviewed changes

DrahtBot added the Needs rebase label Jun 12, 2023

brunoerg force-pushed the 2023-01-netprocessing-txn branch from 686629d to 89478af Compare June 12, 2023 13:30

DrahtBot requested review from ajtowns and dergoegge June 12, 2023 13:34

DrahtBot removed the Needs rebase label Jun 12, 2023

ajtowns reviewed Jun 15, 2023

View reviewed changes

src/net_processing.cpp Outdated Show resolved Hide resolved

brunoerg force-pushed the 2023-01-netprocessing-txn branch from 89478af to 77d6d89 Compare June 15, 2023 15:23

DrahtBot requested a review from ajtowns June 15, 2023 15:50

DrahtBot removed the request for review from ajtowns June 23, 2023 14:28

achow101 merged commit 50a664a into bitcoin:master Jun 23, 2023

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 25, 2023

Merge bitcoin#26969: net, refactor: net_processing, add `ProcessCompa…

52016db

…ctBlock…

bitcoin locked and limited conversation to collaborators Jun 22, 2024

	if (fAlreadyInFlight) {
	// We requested this block, but its far into the future, so our
	// mempool will probably be useless - request the block normally
	std::vector<CInv> vInv(1);
	vInv[0] = CInv(MSG_BLOCK \| GetFetchFlags(*peer), cmpctblock.header.GetHash());
	m_connman.PushMessage(&pfrom, msgMaker.Make(NetMsgType::GETDATA, vInv));
	return;
	} else {
	// If this was an announce-cmpctblock, we want the same treatment as a header message
	fRevertToHeaderProcessing = true;
	}
	}
	} // cs_main

	if (fProcessBLOCKTXN) {
	return ProcessMessage(pfrom, NetMsgType::BLOCKTXN, blockTxnMsg, time_received, interruptMsgProc);
	}

net, refactor: net_processing, add ProcessCompactBlockTxns #26969

net, refactor: net_processing, add ProcessCompactBlockTxns #26969

Uh oh!

Conversation

brunoerg commented Jan 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jan 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

dergoegge commented Jan 25, 2023

Uh oh!

Uh oh!

brunoerg commented Jan 26, 2023

Uh oh!

Uh oh!

brunoerg commented Jan 27, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brunoerg commented Mar 2, 2023

Uh oh!

brunoerg commented Mar 21, 2023

Uh oh!

ajtowns commented Jun 6, 2023

Uh oh!

Uh oh!

instagibbs commented Jun 7, 2023

Uh oh!

brunoerg commented Jun 7, 2023

Uh oh!

ajtowns commented Jun 8, 2023

Uh oh!

dergoegge left a comment

Choose a reason for hiding this comment

Uh oh!

brunoerg commented Jun 12, 2023

Uh oh!

instagibbs commented Jun 12, 2023

Uh oh!

ajtowns left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brunoerg commented Jun 15, 2023

Uh oh!

instagibbs commented Jun 15, 2023

Uh oh!

ajtowns commented Jun 23, 2023

Uh oh!

achow101 commented Jun 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

net, refactor: net_processing, add `ProcessCompactBlockTxns` #26969

net, refactor: net_processing, add `ProcessCompactBlockTxns` #26969

brunoerg commented Jan 25, 2023 •

edited

Loading

DrahtBot commented Jan 25, 2023 •

edited

Loading