diff --git a/qa/rpc-tests/p2p-compactblocks.py b/qa/rpc-tests/p2p-compactblocks.py index ab4b809ded..fc1f16c6d2 100755 --- a/qa/rpc-tests/p2p-compactblocks.py +++ b/qa/rpc-tests/p2p-compactblocks.py @@ -764,6 +764,54 @@ class CompactBlocksTest(BitcoinTestFramework): msg.announce = True peer.send_and_ping(msg) + def test_compactblock_reconstruction_multiple_peers(self, node, stalling_peer, delivery_peer): + assert(len(self.utxos)) + + def announce_cmpct_block(node, peer): + utxo = self.utxos.pop(0) + block = self.build_block_with_transactions(node, utxo, 5) + + cmpct_block = HeaderAndShortIDs() + cmpct_block.initialize_from_block(block) + msg = msg_cmpctblock(cmpct_block.to_p2p()) + peer.send_and_ping(msg) + with mininode_lock: + assert(peer.last_getblocktxn is not None) + return block, cmpct_block + + block, cmpct_block = announce_cmpct_block(node, stalling_peer) + + for tx in block.vtx[1:]: + delivery_peer.send_message(msg_tx(tx)) + delivery_peer.sync_with_ping() + mempool = node.getrawmempool() + for tx in block.vtx[1:]: + assert(tx.hash in mempool) + + delivery_peer.send_and_ping(msg_cmpctblock(cmpct_block.to_p2p())) + assert_equal(int(node.getbestblockhash(), 16), block.sha256) + + self.utxos.append([block.vtx[-1].sha256, 0, block.vtx[-1].vout[0].nValue]) + + # Now test that delivering an invalid compact block won't break relay + + block, cmpct_block = announce_cmpct_block(node, stalling_peer) + for tx in block.vtx[1:]: + delivery_peer.send_message(msg_tx(tx)) + delivery_peer.sync_with_ping() + + cmpct_block.prefilled_txn[0].tx.wit.vtxinwit = [ CTxInWitness() ] + cmpct_block.prefilled_txn[0].tx.wit.vtxinwit[0].scriptWitness.stack = [ser_uint256(0)] + + cmpct_block.use_witness = True + delivery_peer.send_and_ping(msg_cmpctblock(cmpct_block.to_p2p())) + assert(int(node.getbestblockhash(), 16) != block.sha256) + + msg = msg_blocktxn() + msg.block_transactions.blockhash = block.sha256 + msg.block_transactions.transactions = block.vtx[1:] + stalling_peer.send_and_ping(msg) + assert_equal(int(node.getbestblockhash(), 16), block.sha256) def run_test(self): # Setup the p2p connections and start up the network thread. @@ -848,6 +896,10 @@ class CompactBlocksTest(BitcoinTestFramework): self.test_invalid_tx_in_compactblock(self.nodes[1], self.segwit_node, False) self.test_invalid_tx_in_compactblock(self.nodes[1], self.old_node, False) + print("\tTesting reconstructing compact blocks from all peers...") + self.test_compactblock_reconstruction_multiple_peers(self.nodes[1], self.segwit_node, self.old_node) + sync_blocks(self.nodes) + # Advance to segwit activation print ("\nAdvancing to segwit activation\n") self.activate_segwit(self.nodes[1]) diff --git a/src/net_processing.cpp b/src/net_processing.cpp index a4e5ad2095..e10694ed18 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1782,6 +1782,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } } + // Keep a CBlock for "optimistic" compactblock reconstructions (see + // below) + std::shared_ptr pblock = std::make_shared(); + bool fBlockReconstructed = false; + LOCK(cs_main); // If AcceptBlockHeader returned true, it set pindex assert(pindex); @@ -1870,6 +1875,23 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, req.blockhash = pindex->GetBlockHash(); connman.PushMessage(pfrom, msgMaker.Make(NetMsgType::GETBLOCKTXN, req)); } + } else { + // This block is either already in flight from a different + // peer, or this peer has too many blocks outstanding to + // download from. + // Optimistically try to reconstruct anyway since we might be + // able to without any round trips. + PartiallyDownloadedBlock tempBlock(&mempool); + ReadStatus status = tempBlock.InitData(cmpctblock); + if (status != READ_STATUS_OK) { + // TODO: don't ignore failures + return true; + } + std::vector dummy; + status = tempBlock.FillBlock(*pblock, dummy); + if (status == READ_STATUS_OK) { + fBlockReconstructed = true; + } } } else { if (fAlreadyInFlight) { @@ -1889,6 +1911,29 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, return ProcessMessage(pfrom, NetMsgType::HEADERS, vHeadersMsg, nTimeReceived, chainparams, connman); } } + + if (fBlockReconstructed) { + // If we got here, we were able to optimistically reconstruct a + // block that is in flight from some other peer. + { + LOCK(cs_main); + mapBlockSource.emplace(pblock->GetHash(), std::make_pair(pfrom->GetId(), false)); + } + bool fNewBlock = false; + ProcessNewBlock(chainparams, pblock, true, &fNewBlock); + if (fNewBlock) + pfrom->nLastBlockTime = GetTime(); + + LOCK(cs_main); // hold cs_main for CBlockIndex::IsValid() + if (pindex->IsValid(BLOCK_VALID_TRANSACTIONS)) { + // Clear download state for this block, which is in + // process from some other peer. We do this after calling + // ProcessNewBlock so that a malleated cmpctblock announcement + // can't be used to interfere with block relay. + MarkBlockAsReceived(pblock->GetHash()); + } + } + } else if (strCommand == NetMsgType::BLOCKTXN && !fImporting && !fReindex) // Ignore blocks received while importing