diff options
| -rwxr-xr-x | qa/rpc-tests/p2p-compactblocks.py | 52 | ||||
| -rw-r--r-- | src/main.cpp | 49 |
2 files changed, 101 insertions, 0 deletions
diff --git a/qa/rpc-tests/p2p-compactblocks.py b/qa/rpc-tests/p2p-compactblocks.py index e0b72e6840..156a559b10 100755 --- a/qa/rpc-tests/p2p-compactblocks.py +++ b/qa/rpc-tests/p2p-compactblocks.py @@ -757,6 +757,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. @@ -841,6 +889,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/main.cpp b/src/main.cpp index f10cce2422..e2fc7d1b12 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -5650,6 +5650,11 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, CBlockHeaderAndShortTxIDs cmpctblock; vRecv >> cmpctblock; + // Keep a CBlock for "optimistic" compactblock reconstructions (see + // below) + CBlock block; + bool fBlockReconstructed = false; + LOCK(cs_main); if (mapBlockIndex.find(cmpctblock.header.hashPrevBlock) == mapBlockIndex.end()) { @@ -5758,6 +5763,23 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, req.blockhash = pindex->GetBlockHash(); pfrom->PushMessage(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<CTransaction> dummy; + status = tempBlock.FillBlock(block, dummy); + if (status == READ_STATUS_OK) { + fBlockReconstructed = true; + } } } else { if (fAlreadyInFlight) { @@ -5778,6 +5800,33 @@ bool static ProcessMessage(CNode* pfrom, string strCommand, CDataStream& vRecv, } } + if (fBlockReconstructed) { + // If we got here, we were able to optimistically reconstruct a + // block that is in flight from some other peer. However, this + // cmpctblock may be invalid. In particular, while we've checked + // that the block merkle root commits to the transaction ids, we + // haven't yet checked that tx witnesses are properly committed to + // in the coinbase witness commitment. + // + // ProcessNewBlock will call MarkBlockAsReceived(), which will + // clear any in-flight compact block state that might be present + // from some other peer. We don't want a malleated compact block + // request to interfere with block relay, so we don't want to call + // ProcessNewBlock until we've already checked that the witness + // commitment is correct. + { + LOCK(cs_main); + CValidationState dummy; + if (!ContextualCheckBlock(block, dummy, pindex->pprev)) { + // TODO: could send reject message to peer? + return true; + } + } + CValidationState state; + ProcessNewBlock(state, chainparams, pfrom, &block, true, NULL, false); + // TODO: could send reject message if block is invalid? + } + CheckBlockIndex(chainparams.GetConsensus()); } |