Changeset View
Changeset View
Standalone View
Standalone View
src/net_processing.cpp
Show First 20 Lines • Show All 2,972 Lines • ▼ Show 20 Lines | if (msg_type == NetMsgType::VERACK) { | ||||
bool fAnnounceUsingCMPCTBLOCK = false; | bool fAnnounceUsingCMPCTBLOCK = false; | ||||
uint64_t nCMPCTBLOCKVersion = 1; | uint64_t nCMPCTBLOCKVersion = 1; | ||||
m_connman.PushMessage(&pfrom, | m_connman.PushMessage(&pfrom, | ||||
msgMaker.Make(NetMsgType::SENDCMPCT, | msgMaker.Make(NetMsgType::SENDCMPCT, | ||||
fAnnounceUsingCMPCTBLOCK, | fAnnounceUsingCMPCTBLOCK, | ||||
nCMPCTBLOCKVersion)); | nCMPCTBLOCKVersion)); | ||||
} | } | ||||
if ((pfrom.nServices & NODE_AVALANCHE) && g_avalanche && | if (g_avalanche && | ||||
gArgs.GetBoolArg("-enableavalanche", AVALANCHE_DEFAULT_ENABLED)) { | gArgs.GetBoolArg("-enableavalanche", AVALANCHE_DEFAULT_ENABLED)) { | ||||
if (g_avalanche->sendHello(&pfrom)) { | for (const avalanche::Peer &peer : g_avalanche->getPeers()) { | ||||
deadalnix: It's literally impossible that this is thread safe.
Also, as per your previous comment, this… | |||||
deadalnixUnsubmitted Not Done Inline ActionsUpon further digging, this is thread safe because it creates a full copy of the whole peer list, as per D9290 . I'm not sure if this better or worse. This clearly shows the need for an inversion of control. Ditch getPeers, and add a way to iterrate over all peers using a lambda, in CConMan::ForEachNode . deadalnix: Upon further digging, this is thread safe because it creates a full copy of the whole peer list… | |||||
pfrom.PushProofInventory(peer.proof.getId()); | |||||
} | |||||
if ((pfrom.nServices & NODE_AVALANCHE) && | |||||
g_avalanche->sendHello(&pfrom)) { | |||||
LogPrint(BCLog::NET, "Send avahello to peer %d\n", | LogPrint(BCLog::NET, "Send avahello to peer %d\n", | ||||
pfrom.GetId()); | pfrom.GetId()); | ||||
} | } | ||||
} | } | ||||
pfrom.fSuccessfullyConnected = true; | pfrom.fSuccessfullyConnected = true; | ||||
return; | return; | ||||
} | } | ||||
▲ Show 20 Lines • Show All 2,196 Lines • ▼ Show 20 Lines | if (pingSend) { | ||||
// Message: inventory | // Message: inventory | ||||
// | // | ||||
std::vector<CInv> vInv; | std::vector<CInv> vInv; | ||||
{ | { | ||||
auto addInvAndMaybeFlush = [&](uint32_t type, const uint256 &hash) { | auto addInvAndMaybeFlush = [&](uint32_t type, const uint256 &hash) { | ||||
vInv.emplace_back(type, hash); | vInv.emplace_back(type, hash); | ||||
if (vInv.size() == MAX_INV_SZ) { | if (vInv.size() == MAX_INV_SZ) { | ||||
m_connman.PushMessage(pto, | m_connman.PushMessage(pto, | ||||
msgMaker.Make(NetMsgType::INV, vInv)); | msgMaker.Make(NetMsgType::INV, vInv)); | ||||
deadalnixUnsubmitted Not Done Inline ActionsIt looks like you should move the vector here instead of copying just to clear it after. (I commented on the other diff about this). deadalnix: It looks like you should move the vector here instead of copying just to clear it after. (I… | |||||
vInv.clear(); | vInv.clear(); | ||||
} | } | ||||
}; | }; | ||||
LOCK(pto->cs_inventory); | LOCK(pto->cs_inventory); | ||||
vInv.reserve(std::max<size_t>(pto->vInventoryBlockToSend.size(), | vInv.reserve(std::max<size_t>(pto->vInventoryBlockToSend.size(), | ||||
INVENTORY_BROADCAST_MAX_PER_MB * | INVENTORY_BROADCAST_MAX_PER_MB * | ||||
config.GetMaxBlockSize() / | config.GetMaxBlockSize() / | ||||
▲ Show 20 Lines • Show All 149 Lines • ▼ Show 20 Lines | if (pingSend) { | ||||
ret.first)); | ret.first)); | ||||
} | } | ||||
} | } | ||||
addInvAndMaybeFlush(MSG_TX, txid); | addInvAndMaybeFlush(MSG_TX, txid); | ||||
pto->m_tx_relay->filterInventoryKnown.insert(txid); | pto->m_tx_relay->filterInventoryKnown.insert(txid); | ||||
} | } | ||||
} | } | ||||
} | } | ||||
// Add proofs to inventory | |||||
deadalnixUnsubmitted Not Done Inline ActionsIn term of priority, shouldn't this go in between blocks and transactions? deadalnix: In term of priority, shouldn't this go in between blocks and transactions? | |||||
if (pto->m_proof_relay != nullptr) { | |||||
LOCK(pto->m_proof_relay->cs_proof_inventory); | |||||
auto it = pto->m_proof_relay->setInventoryProofToSend.begin(); | |||||
while (it != | |||||
pto->m_proof_relay->setInventoryProofToSend.end()) { | |||||
addInvAndMaybeFlush(MSG_AVA_PROOF, *it); | |||||
it = pto->m_proof_relay->setInventoryProofToSend.erase(it); | |||||
} | |||||
} | |||||
} | } | ||||
if (!vInv.empty()) { | if (!vInv.empty()) { | ||||
m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); | m_connman.PushMessage(pto, msgMaker.Make(NetMsgType::INV, vInv)); | ||||
} | } | ||||
// Detect whether we're stalling | // Detect whether we're stalling | ||||
current_time = GetTime<std::chrono::microseconds>(); | current_time = GetTime<std::chrono::microseconds>(); | ||||
// nNow is the current system time (GetTimeMicros is not mockable) and | // nNow is the current system time (GetTimeMicros is not mockable) and | ||||
// should be replaced by the mockable current_time eventually | // should be replaced by the mockable current_time eventually | ||||
▲ Show 20 Lines • Show All 259 Lines • Show Last 20 Lines |
It's literally impossible that this is thread safe.
Also, as per your previous comment, this doesn't seem like a reasonable thing to do anyways. This is creating a quadratic behavior in the number of proof (more proof, mean more node supporting proofs for which we need to do an ops that is linear in the number of proofs).
This is simply not a good idea.