Repository: bitcoin/bitcoin
File: src/net_processing.cpp (C++)

Code before:
        bool first_in_flight = already_in_flight == 0 || (range_flight.first->second.first == pfrom.GetId());

        while (range_flight.first != range_flight.second) {
            auto [node_id, block_it] = range_flight.first->second;
            if (node_id == pfrom.GetId() && block_it->partialBlock) {
                requested_block_from_this_peer = true;
                break;
            }
            range_flight.first++;
        }

        if (!requested_block_from_this_peer) {
            LogDebug(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId());
            return;
        }

        PartiallyDownloadedBlock& partialBlock = *range_flight.first->second.second->partialBlock;

        // We should not have gotten this far in compact block processing unless it's attached to a known header
        const CBlockIndex* prev_block{Assume(m_chainman.m_blockman.LookupBlockIndex(partialBlock.header.hashPrevBlock))};
        ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn,
                                                   /*segwit_active=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT));
        if (status == READ_STATUS_INVALID) {
            RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect
            Misbehaving(peer, "invalid compact block/non-matching block transactions");
            return;
        } else if (status == READ_STATUS_FAILED) {
            if (first_in_flight) {
                // Might have collided, fall back to getdata now :(
                std::vector<CInv> invs;
                invs.emplace_back(MSG_BLOCK | GetFetchFlags(peer), block_transactions.blockhash);
                MakeAndPushMessage(pfrom, NetMsgType::GETDATA, invs);
            } else {
                RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId());
                LogDebug(BCLog::NET, "Peer %d sent us a compact block but it failed to reconstruct, waiting on first download to complete\n", pfrom.GetId());
                return;
            }
        } else {
            // Block is okay for further processing
            RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // it is now an empty pointer
            fBlockRead = true;
            // mapBlockSource is used for potentially punishing peers and
            // updating which peers send us compact blocks, so the race
            // between here and cs_main in ProcessNewBlock is fine.
            // BIP 152 permits peers to relay compact blocks after validating
            // the header only; we should not punish peers if the block turns
            // out to be invalid.
            mapBlockSource.emplace(block_transactions.blockhash, std::make_pair(pfrom.GetId(), false));
        }
    } // Don't hold cs_main when we call into ProcessNewBlock
    if (fBlockRead) {

Review comment (bug):
I think this will change behavior in non-debug builds such that repeated `blocktxn` messages will now be ignored (whereas they would result in `READ_STATUS_INVALID` / disconnection before). Shouldn't we rather keep on disconnecting the peer?

Code after:
        bool first_in_flight = already_in_flight == 0 || (range_flight.first->second.first == pfrom.GetId());

        while (range_flight.first != range_flight.second) {
            auto [node_id, block_it] = range_flight.first->second;
            if (node_id == pfrom.GetId() && block_it->partialBlock) {
                requested_block_from_this_peer = true;
                break;
            }
            range_flight.first++;
        }

        if (!requested_block_from_this_peer) {
            LogDebug(BCLog::NET, "Peer %d sent us block transactions for block we weren't expecting\n", pfrom.GetId());
            return;
        }

        PartiallyDownloadedBlock& partialBlock = *range_flight.first->second.second->partialBlock;

        if (partialBlock.header.IsNull()) {
            // It is possible for the header to be empty if a previous call to FillBlock wiped the header, but left
            // the PartiallyDownloadedBlock pointer around (i.e. did not call RemoveBlockRequest). In this case, we
            // should not call LookupBlockIndex below.
            RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId());
            Misbehaving(peer, "previous compact block reconstruction attempt failed");
            LogDebug(BCLog::NET, "Peer %d sent compact block transactions multiple times", pfrom.GetId());
            return;
        }

        // We should not have gotten this far in compact block processing unless it's attached to a known header
        const CBlockIndex* prev_block{Assume(m_chainman.m_blockman.LookupBlockIndex(partialBlock.header.hashPrevBlock))};
        ReadStatus status = partialBlock.FillBlock(*pblock, block_transactions.txn,
                                                   /*segwit_active=*/DeploymentActiveAfter(prev_block, m_chainman, Consensus::DEPLOYMENT_SEGWIT));
        if (status == READ_STATUS_INVALID) {
            RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId()); // Reset in-flight state in case Misbehaving does not result in a disconnect
            Misbehaving(peer, "invalid compact block/non-matching block transactions");
            return;
        } else if (status == READ_STATUS_FAILED) {
            if (first_in_flight) {
                // Might have collided, fall back to getdata now :(
                // We keep the failed partialBlock to disallow processing another compact block announcement from the same
                // peer for the same block. We let the full block download below continue under the same m_downloading_since
                // timer.
                std::vector<CInv> invs;
                invs.emplace_back(MSG_BLOCK | GetFetchFlags(peer), block_transactions.blockhash);
                MakeAndPushMessage(pfrom, NetMsgType::GETDATA, invs);
            } else {
                RemoveBlockRequest(block_transactions.blockhash, pfrom.GetId());
                LogDebug(BCLog::NET, "Peer %d sent us a compact block but it failed to reconstruct, waiting on first download to complete\n", pfrom.GetId());
                return;
            }
        } else {